diff mbox

printk-formats.txt: Add examples for %pS and %pF

Message ID 20170810173533.GA11600@ls3530.fritz.box (mailing list archive)
State Superseded
Headers show

Commit Message

Helge Deller Aug. 10, 2017, 5:35 p.m. UTC 
Sometimes people seems unclear when to use the %pS or %pF printk format.
Adding some examples may help to avoid such mistakes.

See for example commit 51d96dc2e2dc ("random: fix warning message on ia64 and
parisc") which fixed such a wrong format string.

Signed-off-by: Helge Deller <deller@gmx.de>

--
To unsubscribe from this list: send the line "unsubscribe linux-parisc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Sergey Senozhatsky Aug. 11, 2017, 12:15 a.m. UTC | #1 
On (08/10/17 19:35), Helge Deller wrote:
> Sometimes people seems unclear when to use the %pS or %pF printk format.
> Adding some examples may help to avoid such mistakes.
> 
> See for example commit 51d96dc2e2dc ("random: fix warning message on ia64 and
> parisc") which fixed such a wrong format string.
> 
> Signed-off-by: Helge Deller <deller@gmx.de>
> 
> diff --git a/Documentation/printk-formats.txt b/Documentation/printk-formats.txt
> index 65ea591..be8c05b 100644
> --- a/Documentation/printk-formats.txt
> +++ b/Documentation/printk-formats.txt
> @@ -73,6 +73,12 @@ actually function descriptors which must first be resolved. The ``F`` and
>  ``f`` specifiers perform this resolution and then provide the same
>  functionality as the ``S`` and ``s`` specifiers.
>  
> +Examples::
> +
> +	printk("Called from %pS.\n", __builtin_return_address(0));
> +	printk("Called from %pS.\n", (void *)regs->ip);
> +	printk("Called from %pF.\n", &gettimeofday);

sorry, but how does it help?


there is this paragraph

: On ia64, ppc64 and parisc64 architectures function pointers are
: actually function descriptors which must first be resolved. The ``F`` and
: ``f`` specifiers perform this resolution and then provide the same
: functionality as the ``S`` and ``s`` specifiers.

which supposed to explain everything in details. the examples
don't make it any `clearer', IMHO.


*may be* on "ia64, ppc64 and parisc64" we can somehow check
that the pointer, which we pass as %pS, belongs to .text and
print some build-time warnings. well, if it's actually a
problem. dunno.

	-ss
--
To unsubscribe from this list: send the line "unsubscribe linux-parisc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Helge Deller Aug. 11, 2017, 7:31 a.m. UTC | #2 
On 11.08.2017 02:15, Sergey Senozhatsky wrote:
> On (08/10/17 19:35), Helge Deller wrote:
>> Sometimes people seems unclear when to use the %pS or %pF printk format.
>> Adding some examples may help to avoid such mistakes.
>>
>> See for example commit 51d96dc2e2dc ("random: fix warning message on ia64 and
>> parisc") which fixed such a wrong format string.
>>
>> Signed-off-by: Helge Deller <deller@gmx.de>
>>
>> diff --git a/Documentation/printk-formats.txt b/Documentation/printk-formats.txt
>> index 65ea591..be8c05b 100644
>> --- a/Documentation/printk-formats.txt
>> +++ b/Documentation/printk-formats.txt
>> @@ -73,6 +73,12 @@ actually function descriptors which must first be resolved. The ``F`` and
>>  ``f`` specifiers perform this resolution and then provide the same
>>  functionality as the ``S`` and ``s`` specifiers.
>>  
>> +Examples::
>> +
>> +	printk("Called from %pS.\n", __builtin_return_address(0));
>> +	printk("Called from %pS.\n", (void *)regs->ip);
>> +	printk("Called from %pF.\n", &gettimeofday);
> 
> sorry, but how does it help?
> 
> 
> there is this paragraph
> 
> : On ia64, ppc64 and parisc64 architectures function pointers are
> : actually function descriptors which must first be resolved. The ``F`` and
> : ``f`` specifiers perform this resolution and then provide the same
> : functionality as the ``S`` and ``s`` specifiers.
> 
> which supposed to explain everything in details. the examples
> don't make it any `clearer', IMHO.

Experts surely do know what function descriptors are.
Nevertheless even those often get it wrong as can be seen in
various commits.

The hope with this patch is to show widely-used examples
and avoid additional commits afterwards to fix it up.

This patch was meant to be RFC.
If you decide not to take it, I'm fine as well.

> *may be* on "ia64, ppc64 and parisc64" we can somehow check
> that the pointer, which we pass as %pS, belongs to .text and
> print some build-time warnings. well, if it's actually a
> problem. dunno.

I think it's not needed. Those bugs will be seen and fixed.

Helge
--
To unsubscribe from this list: send the line "unsubscribe linux-parisc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Steven Rostedt Aug. 15, 2017, 12:46 p.m. UTC | #3 
On Thu, 10 Aug 2017 19:35:33 +0200
Helge Deller <deller@gmx.de> wrote:

> Sometimes people seems unclear when to use the %pS or %pF printk format.
> Adding some examples may help to avoid such mistakes.
> 
> See for example commit 51d96dc2e2dc ("random: fix warning message on ia64 and
> parisc") which fixed such a wrong format string.
> 
> Signed-off-by: Helge Deller <deller@gmx.de>
> 
> diff --git a/Documentation/printk-formats.txt b/Documentation/printk-formats.txt
> index 65ea591..be8c05b 100644
> --- a/Documentation/printk-formats.txt
> +++ b/Documentation/printk-formats.txt
> @@ -73,6 +73,12 @@ actually function descriptors which must first be resolved. The ``F`` and
>  ``f`` specifiers perform this resolution and then provide the same
>  functionality as the ``S`` and ``s`` specifiers.
>  
> +Examples::
> +
> +	printk("Called from %pS.\n", __builtin_return_address(0));
> +	printk("Called from %pS.\n", (void *)regs->ip);
> +	printk("Called from %pF.\n", &gettimeofday);

Is the '&' really necessary? What about using the example:

	printk("Called in %pF.\n", __func__);

?

-- Steve

> +
>  Kernel Pointers
>  ===============
>  

--
To unsubscribe from this list: send the line "unsubscribe linux-parisc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Helge Deller Aug. 15, 2017, 7:41 p.m. UTC | #4 
On 15.08.2017 14:46, Steven Rostedt wrote:
> On Thu, 10 Aug 2017 19:35:33 +0200
> Helge Deller <deller@gmx.de> wrote:
> 
>> Sometimes people seems unclear when to use the %pS or %pF printk format.
>> Adding some examples may help to avoid such mistakes.
>>
>> See for example commit 51d96dc2e2dc ("random: fix warning message on ia64 and
>> parisc") which fixed such a wrong format string.
>>
>> Signed-off-by: Helge Deller <deller@gmx.de>
>>
>> diff --git a/Documentation/printk-formats.txt b/Documentation/printk-formats.txt
>> index 65ea591..be8c05b 100644
>> --- a/Documentation/printk-formats.txt
>> +++ b/Documentation/printk-formats.txt
>> @@ -73,6 +73,12 @@ actually function descriptors which must first be resolved. The ``F`` and
>>  ``f`` specifiers perform this resolution and then provide the same
>>  functionality as the ``S`` and ``s`` specifiers.
>>  
>> +Examples::
>> +
>> +	printk("Called from %pS.\n", __builtin_return_address(0));
>> +	printk("Called from %pS.\n", (void *)regs->ip);
>> +	printk("Called from %pF.\n", &gettimeofday);
> 
> Is the '&' really necessary? 
The '&' is not necessary. The compiler doesn't complain either.

> What about using the example:
> 	printk("Called in %pF.\n", __func__);

Very interesting!

This code:
void smp_cpus_done() {
printk("Called from %pF.\n", smp_cpus_done);
printk("Called from %pf.\n", smp_cpus_done);
printk("Called in %pS.\n", __func__);
printk("Called in %ps.\n", __func__);
printk("Called in %pF.\n", __func__);
printk("Called in %pf.\n", __func__);

gives:
 Called from smp_cpus_done+0x0/0x1b8.
 Called from smp_cpus_done.
 Called in __func__.28197+0x0/0x20.
 Called in __func__.28197.
 Called in 0x5041524953433332.
 Called in 0x5041524953433332.

So, the correct usage is:
printk("Called in %pS.\n", __func__);

But it should have printed
 Called from smp_cpus_done+0x0/0x1b8.
which means the (parisc?) printk resolver doesn't work correctly.

In assembly code a pointer to this object is handed to printk:
        .type   __func__.28197, @object
        .size   __func__.28197, 14
__func__.28197:
        .stringz        "smp_cpus_done"

I'll look into this problem.

Helge
--
To unsubscribe from this list: send the line "unsubscribe linux-parisc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Helge Deller Aug. 15, 2017, 7:47 p.m. UTC | #5 
On 15.08.2017 21:41, Helge Deller wrote:
> On 15.08.2017 14:46, Steven Rostedt wrote:
>> On Thu, 10 Aug 2017 19:35:33 +0200
>> Helge Deller <deller@gmx.de> wrote:
>>
>>> Sometimes people seems unclear when to use the %pS or %pF printk format.
>>> Adding some examples may help to avoid such mistakes.
>>>
>>> See for example commit 51d96dc2e2dc ("random: fix warning message on ia64 and
>>> parisc") which fixed such a wrong format string.
>>>
>>> Signed-off-by: Helge Deller <deller@gmx.de>
>>>
>>> diff --git a/Documentation/printk-formats.txt b/Documentation/printk-formats.txt
>>> index 65ea591..be8c05b 100644
>>> --- a/Documentation/printk-formats.txt
>>> +++ b/Documentation/printk-formats.txt
>>> @@ -73,6 +73,12 @@ actually function descriptors which must first be resolved. The ``F`` and
>>>  ``f`` specifiers perform this resolution and then provide the same
>>>  functionality as the ``S`` and ``s`` specifiers.
>>>  
>>> +Examples::
>>> +
>>> +	printk("Called from %pS.\n", __builtin_return_address(0));
>>> +	printk("Called from %pS.\n", (void *)regs->ip);
>>> +	printk("Called from %pF.\n", &gettimeofday);
>>
>> Is the '&' really necessary? 
> The '&' is not necessary. The compiler doesn't complain either.
> 
>> What about using the example:
>> 	printk("Called in %pF.\n", __func__);
> 
> Very interesting!
> 
> This code:
> void smp_cpus_done() {
> printk("Called from %pF.\n", smp_cpus_done);
> printk("Called from %pf.\n", smp_cpus_done);
> printk("Called in %pS.\n", __func__);
> printk("Called in %ps.\n", __func__);
> printk("Called in %pF.\n", __func__);
> printk("Called in %pf.\n", __func__);
> 
> gives:
>  Called from smp_cpus_done+0x0/0x1b8.
>  Called from smp_cpus_done.
>  Called in __func__.28197+0x0/0x20.
>  Called in __func__.28197.
>  Called in 0x5041524953433332.
>  Called in 0x5041524953433332.
> 
> So, the correct usage is:
> printk("Called in %pS.\n", __func__);

I'm wrong.
The correct usage would be:
 printk("Called in %s.\n", __func__);

__func__ is just a pointer to a string.

Helge

> 
> But it should have printed
>  Called from smp_cpus_done+0x0/0x1b8.
> which means the (parisc?) printk resolver doesn't work correctly.
> 
> In assembly code a pointer to this object is handed to printk:
>         .type   __func__.28197, @object
>         .size   __func__.28197, 14
> __func__.28197:
>         .stringz        "smp_cpus_done"
> 
> I'll look into this problem.
> 
> Helge
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-parisc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Steven Rostedt Aug. 15, 2017, 9:35 p.m. UTC | #6 
On Tue, 15 Aug 2017 21:47:27 +0200
Helge Deller <deller@gmx.de> wrote:

> > Very interesting!
> > 
> > This code:
> > void smp_cpus_done() {
> > printk("Called from %pF.\n", smp_cpus_done);
> > printk("Called from %pf.\n", smp_cpus_done);
> > printk("Called in %pS.\n", __func__);
> > printk("Called in %ps.\n", __func__);
> > printk("Called in %pF.\n", __func__);
> > printk("Called in %pf.\n", __func__);
> > 
> > gives:
> >  Called from smp_cpus_done+0x0/0x1b8.
> >  Called from smp_cpus_done.
> >  Called in __func__.28197+0x0/0x20.
> >  Called in __func__.28197.
> >  Called in 0x5041524953433332.
> >  Called in 0x5041524953433332.
> > 
> > So, the correct usage is:
> > printk("Called in %pS.\n", __func__);  
> 
> I'm wrong.
> The correct usage would be:
>  printk("Called in %s.\n", __func__);
> 
> __func__ is just a pointer to a string.

OK, I'm still on vacation :-/

Yeah, I was looking for usages of %pF, and came across this:

		pr_warn("%s: NULL omap_sr from %pF\n",
			__func__, (void *)_RET_IP_);

And not noticing the first "%s" :-p

-- Steve

--
To unsubscribe from this list: send the line "unsubscribe linux-parisc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/Documentation/printk-formats.txt b/Documentation/printk-formats.txt
index 65ea591..be8c05b 100644
--- a/Documentation/printk-formats.txt
+++ b/Documentation/printk-formats.txt
@@ -73,6 +73,12 @@  actually function descriptors which must first be resolved. The ``F`` and
 ``f`` specifiers perform this resolution and then provide the same
 functionality as the ``S`` and ``s`` specifiers.
 
+Examples::
+
+	printk("Called from %pS.\n", __builtin_return_address(0));
+	printk("Called from %pS.\n", (void *)regs->ip);
+	printk("Called from %pF.\n", &gettimeofday);
+
 Kernel Pointers
 ===============