From: Gal Pressman <gal@nvidia.com>

Add a new Coccinelle script to identify places where PTR_ERR() is used
in print functions and suggest using the %pe format specifier instead.

For printing error pointers (i.e., a pointer for which IS_ERR() is true)
%pe will print a symbolic error name (e.g,. -EINVAL), opposed to the raw
errno (e.g,. -22) produced by PTR_ERR().
It also makes the code cleaner by saving a redundant call to PTR_ERR().

The script supports context, report, and org modes.

Example transformation:
    printk("Error: %ld\n", PTR_ERR(ptr));  // Before
    printk("Error: %pe\n", ptr);          // After

Signed-off-by: Gal Pressman <gal@nvidia.com>
Reviewed-by: Alexei Lazar <alazar@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
 scripts/coccinelle/misc/ptr_err_to_pe.cocci | 34 +++++++++++++++++++++
 1 file changed, 34 insertions(+)
 create mode 100644 scripts/coccinelle/misc/ptr_err_to_pe.cocci

diff --git a/scripts/coccinelle/misc/ptr_err_to_pe.cocci b/scripts/coccinelle/misc/ptr_err_to_pe.cocci
new file mode 100644
index 000000000000..0494c7709245
--- /dev/null
+++ b/scripts/coccinelle/misc/ptr_err_to_pe.cocci
@@ -0,0 +1,34 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/// Use %pe format specifier instead of PTR_ERR() for printing error pointers.
+///
+/// For printing error pointers (i.e., a pointer for which IS_ERR() is true)
+/// %pe will print a symbolic error name (e.g., -EINVAL), opposed to the raw
+/// errno (e.g., -22) produced by PTR_ERR().
+/// It also makes the code cleaner by saving a redundant call to PTR_ERR().
+///
+// Confidence: High
+// Copyright: (C) 2025 NVIDIA CORPORATION & AFFILIATES.
+// URL: https://coccinelle.gitlabpages.inria.fr/website
+// Options: --no-includes --include-headers
+
+virtual context
+virtual org
+virtual report
+
+@r@
+expression ptr;
+constant fmt;
+position p;
+identifier print_func;
+@@
+* print_func(..., fmt, ..., PTR_ERR@p(ptr), ...)
+
+@script:python depends on r && report@
+p << r.p;
+@@
+coccilib.report.print_report(p[0], "WARNING: Consider using %pe to print PTR_ERR()")
+
+@script:python depends on r && org@
+p << r.p;
+@@
+coccilib.org.print_todo(p[0], "WARNING: Consider using %pe to print PTR_ERR()")
-- 
2.31.1

…> The script supports context, report, and org modes.

I suggest to omit this sentence from the description.


Will the hint “scripts/” be omitted from the patch prefix?


…> +++ b/scripts/coccinelle/misc/ptr_err_to_pe.cocci
…> +// URL: https://coccinelle.gitlabpages.inria.fr/website

I suggest to omit this comment line.


…> +virtual context
> +virtual org
> +virtual report

The restriction on the support for three operation modes will need further development considerations.


> +@r@
> +expression ptr;
> +constant fmt;
> +position p;
> +identifier print_func;
> +@@
> +* print_func(..., fmt, ..., PTR_ERR@p(ptr), ...)

How do you think about to use the metavariable type “format list”?

Would it matter to restrict expressions to pointer expressions?


> +@script:python depends on r && org@

I guess that such an SmPL dependency specification can be simplified a bit.


> +p << r.p;
> +@@
> +coccilib.org.print_todo(p[0], "WARNING: Consider using %pe to print PTR_ERR()")

I suggest to reconsider the implementation detail once more
if the SmPL asterisk functionality fits really to the operation modes “org” and “report”.

The operation mode “context” can usually work also without an extra position variable,
can't it?

Regards,
Markus

Hi Markus!

Thanks for the review, forgive for being a total noob at coccinelle,
hence the many questions.

On 25/09/2025 18:07, Markus Elfring wrote:
> …> The script supports context, report, and org modes.
> 
> I suggest to omit this sentence from the description.
> 
> 
> Will the hint “scripts/” be omitted from the patch prefix?

The patch was merged already, so I'm skipping to comments that can be
fixed in a followup patch.

> 
> 
> …> +++ b/scripts/coccinelle/misc/ptr_err_to_pe.cocci
> …> +// URL: https://coccinelle.gitlabpages.inria.fr/website
> 
> I suggest to omit this comment line.

Sure.

> 
> 
> …> +virtual context
>> +virtual org
>> +virtual report
> 
> The restriction on the support for three operation modes will need further development considerations.

I don't understand what you mean?

> 
> 
>> +@r@
>> +expression ptr;
>> +constant fmt;
>> +position p;
>> +identifier print_func;
>> +@@
>> +* print_func(..., fmt, ..., PTR_ERR@p(ptr), ...)
> 
> How do you think about to use the metavariable type “format list”?

I did find "format list" in the documentation, but spatch fails when I
try to use it.

> 
> Would it matter to restrict expressions to pointer expressions?

I tried changing 'expression ptr;' -> 'expression *ptr;', but then it
didn't find anything. Am I doing it wrong?

> 
> 
>> +@script:python depends on r && org@
> 
> I guess that such an SmPL dependency specification can be simplified a bit.

You mean drop the depends on r?

> 
> 
>> +p << r.p;
>> +@@
>> +coccilib.org.print_todo(p[0], "WARNING: Consider using %pe to print PTR_ERR()")
> 
> I suggest to reconsider the implementation detail once more
> if the SmPL asterisk functionality fits really to the operation modes “org” and “report”.
> 
> The operation mode “context” can usually work also without an extra position variable,
> can't it?

Can you please explain?

> >> +@r@
> >> +expression ptr;
> >> +constant fmt;
> >> +position p;
> >> +identifier print_func;
> >> +@@
> >> +* print_func(..., fmt, ..., PTR_ERR@p(ptr), ...)
> >
> > How do you think about to use the metavariable type “format list”?
>
> I did find "format list" in the documentation, but spatch fails when I
> try to use it.

I would suggest constant char[] fmt.

format is for the case where you want to specify something about the %d
%s, etc in the string.

> > Would it matter to restrict expressions to pointer expressions?
>
> I tried changing 'expression ptr;' -> 'expression *ptr;', but then it
> didn't find anything. Am I doing it wrong?

expression *ptr should be a valid metavariable declaration.  But
Coccinelle needs to have enough information to know that something is a
pointer.  If you have code like a->b and you don't have the definition of
the structure type of a, then it won't know the type of a->b.  More
information about types is available if you use options like
--recursive-includes, but then treatment of every C file will entail
parsing lots of header files, which could make things very slow.  So you
have to consider whether the information that the thing is a pointer is
really necessary to what you are trying to do.

> >> +@script:python depends on r && org@
> >
> > I guess that such an SmPL dependency specification can be simplified a bit.
>
> You mean drop the depends on r?
>
> >
> >
> >> +p << r.p;

Since you have r.p, the rule will only be applied if r has succeeded and
furthermore if p has a value.  So depends on r is not necessary.

julia

> >> +@@
> >> +coccilib.org.print_todo(p[0], "WARNING: Consider using %pe to print PTR_ERR()")
> >
> > I suggest to reconsider the implementation detail once more
> > if the SmPL asterisk functionality fits really to the operation modes “org” and “report”.
> >
> > The operation mode “context” can usually work also without an extra position variable,
> > can't it?
>
> Can you please explain?
>

Thanks for the review Julia!

On 28/09/2025 15:23, Julia Lawall wrote:
>>>> +@r@
>>>> +expression ptr;
>>>> +constant fmt;
>>>> +position p;
>>>> +identifier print_func;
>>>> +@@
>>>> +* print_func(..., fmt, ..., PTR_ERR@p(ptr), ...)
>>>
>>> How do you think about to use the metavariable type “format list”?
>>
>> I did find "format list" in the documentation, but spatch fails when I
>> try to use it.
> 
> I would suggest constant char[] fmt.

That works, thanks!

> 
> format is for the case where you want to specify something about the %d
> %s, etc in the string.
> 
>>> Would it matter to restrict expressions to pointer expressions?
>>
>> I tried changing 'expression ptr;' -> 'expression *ptr;', but then it
>> didn't find anything. Am I doing it wrong?
> 
> expression *ptr should be a valid metavariable declaration.  But
> Coccinelle needs to have enough information to know that something is a
> pointer.  If you have code like a->b and you don't have the definition of
> the structure type of a, then it won't know the type of a->b.  More
> information about types is available if you use options like
> --recursive-includes, but then treatment of every C file will entail
> parsing lots of header files, which could make things very slow.  So you
> have to consider whether the information that the thing is a pointer is
> really necessary to what you are trying to do.

Makes sense, indeed the pointer is embedded in another struct.

I'll keep it as is, if the code calls PTR_ERR() on something that is not
a pointer it has bigger problems than using %pe.

> 
>>>> +@script:python depends on r && org@
>>>
>>> I guess that such an SmPL dependency specification can be simplified a bit.
>>
>> You mean drop the depends on r?
>>
>>>
>>>
>>>> +p << r.p;
> 
> Since you have r.p, the rule will only be applied if r has succeeded and
> furthermore if p has a value.  So depends on r is not necessary.

Got it.

>>> +virtual context
>>> +virtual org
>>> +virtual report
>>
>> The restriction on the support for three operation modes will need further development considerations.
> 
> I don't understand what you mean?

The development status might be unclear for the handling of a varying number of operation modes
by coccicheck rules, isn't it?


> I did find "format list" in the documentation, but spatch fails when I
> try to use it.

Which SmPL code variations did you try out?


>> Would it matter to restrict expressions to pointer expressions?
> 
> I tried changing 'expression ptr;' -> 'expression *ptr;', but then it
> didn't find anything. Am I doing it wrong?

Further software improvements can be reconsidered accordingly.


>>> +@script:python depends on r && org@
>>
>> I guess that such an SmPL dependency specification can be simplified a bit.
> 
> You mean drop the depends on r?

You may omit “r &&” (because the rule is referenced by an SmPL variable declaration),
can't you?


>>> +p << r.p;
>>> +@@
>>> +coccilib.org.print_todo(p[0], "WARNING: Consider using %pe to print PTR_ERR()")
>>
>> I suggest to reconsider the implementation detail once more
>> if the SmPL asterisk functionality fits really to the operation modes “org” and “report”.
>>
>> The operation mode “context” can usually work also without an extra position variable,
>> can't it?
> 
> Can you please explain?

Are you aware of data format requirements here?

Regards,
Markus

On 28/09/2025 15:00, Markus Elfring wrote:
>>>> +virtual context
>>>> +virtual org
>>>> +virtual report
>>>
>>> The restriction on the support for three operation modes will need further development considerations.
>>
>> I don't understand what you mean?
> 
> The development status might be unclear for the handling of a varying number of operation modes
> by coccicheck rules, isn't it?

I'm sorry, I still don't understand what you mean (the problem is likely
on my side).
Do you want me to change anything?

> 
> 
>> I did find "format list" in the documentation, but spatch fails when I
>> try to use it.
> 
> Which SmPL code variations did you try out?

Ended up with Julia's suggestion.

> 
> 
>>> Would it matter to restrict expressions to pointer expressions?
>>
>> I tried changing 'expression ptr;' -> 'expression *ptr;', but then it
>> didn't find anything. Am I doing it wrong?
> 
> Further software improvements can be reconsidered accordingly.
> 
> 
>>>> +@script:python depends on r && org@
>>>
>>> I guess that such an SmPL dependency specification can be simplified a bit.
>>
>> You mean drop the depends on r?
> 
> You may omit “r &&” (because the rule is referenced by an SmPL variable declaration),
> can't you?

Yes, thanks.

> 
> 
>>>> +p << r.p;
>>>> +@@
>>>> +coccilib.org.print_todo(p[0], "WARNING: Consider using %pe to print PTR_ERR()")
>>>
>>> I suggest to reconsider the implementation detail once more
>>> if the SmPL asterisk functionality fits really to the operation modes “org” and “report”.
>>>
>>> The operation mode “context” can usually work also without an extra position variable,
>>> can't it?
>>
>> Can you please explain?
> 
> Are you aware of data format requirements here?

Apparently not, I'll be glad to learn.

>>>>> +virtual context
>>>>> +virtual org
>>>>> +virtual report
>>>>
>>>> The restriction on the support for three operation modes will need further development considerations.
>>>
>>> I don't understand what you mean?
>>
>> The development status might be unclear for the handling of a varying number of operation modes
>> by coccicheck rules, isn't it?
> 
> I'm sorry, I still don't understand what you mean (the problem is likely
> on my side).

The development status is evolving somehow.


> Do you want me to change anything?

You would like to achieve further software refinements.
Did you notice remaining open issues from public information sources?


>>>>> +p << r.p;
>>>>> +@@
>>>>> +coccilib.org.print_todo(p[0], "WARNING: Consider using %pe to print PTR_ERR()")
>>>>
>>>> I suggest to reconsider the implementation detail once more
>>>> if the SmPL asterisk functionality fits really to the operation modes “org” and “report”.
>>>>
>>>> The operation mode “context” can usually work also without an extra position variable,
>>>> can't it?
>>>
>>> Can you please explain?
>>
>> Are you aware of data format requirements here?
> 
> Apparently not, I'll be glad to learn.

Each “operation mode” is connected with a known data format.
The corresponding software documentation is probably still improvable.
Can you determine data format distinctions from published coccicheck scripts
(and related development discussions)?

Regards,
Markus

On 28/09/2025 17:16, Markus Elfring wrote:
>>>>>> +virtual context
>>>>>> +virtual org
>>>>>> +virtual report
>>>>>
>>>>> The restriction on the support for three operation modes will need further development considerations.
>>>>
>>>> I don't understand what you mean?
>>>
>>> The development status might be unclear for the handling of a varying number of operation modes
>>> by coccicheck rules, isn't it?
>>
>> I'm sorry, I still don't understand what you mean (the problem is likely
>> on my side).
> 
> The development status is evolving somehow.
> 
> 
>> Do you want me to change anything?
> 
> You would like to achieve further software refinements.
> Did you notice remaining open issues from public information sources?
> 
> 
>>>>>> +p << r.p;
>>>>>> +@@
>>>>>> +coccilib.org.print_todo(p[0], "WARNING: Consider using %pe to print PTR_ERR()")
>>>>>
>>>>> I suggest to reconsider the implementation detail once more
>>>>> if the SmPL asterisk functionality fits really to the operation modes “org” and “report”.
>>>>>
>>>>> The operation mode “context” can usually work also without an extra position variable,
>>>>> can't it?
>>>>
>>>> Can you please explain?
>>>
>>> Are you aware of data format requirements here?
>>
>> Apparently not, I'll be glad to learn.
> 
> Each “operation mode” is connected with a known data format.
> The corresponding software documentation is probably still improvable.
> Can you determine data format distinctions from published coccicheck scripts
> (and related development discussions)?
> 
> Regards,
> Markus

I'm not really sure if I'm speaking to a real person or some kind of
weird AI, so I'm going to respectfully ignore these comments..

> I'm not really sure if I'm speaking to a real person or some kind of
> weird AI, so I'm going to respectfully ignore these comments..

Can you get more development ideas from another information source
than from hints by known contributors (like me)?
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/dev-tools/coccinelle.rst?h=v6.17-rc7#n71

I hope that the clarification can become more constructive again.

Regards,
Markus

On Thu, 18 Sep 2025 13:43:46 +0300 Tariq Toukan wrote:
> Add a new Coccinelle script to identify places where PTR_ERR() is used
> in print functions and suggest using the %pe format specifier instead.
> 
> For printing error pointers (i.e., a pointer for which IS_ERR() is true)
> %pe will print a symbolic error name (e.g,. -EINVAL), opposed to the raw
> errno (e.g,. -22) produced by PTR_ERR().
> It also makes the code cleaner by saving a redundant call to PTR_ERR().
> 
> The script supports context, report, and org modes.
> 
> Example transformation:
>     printk("Error: %ld\n", PTR_ERR(ptr));  // Before
>     printk("Error: %pe\n", ptr);          // After

Hi Julia, Nicolas,

would you be willing to give us a review tag for this script?
Would you prefer to take the script via your tree?

https://lore.kernel.org/all/1758192227-701925-2-git-send-email-tariqt@nvidia.com/

On Thu, Sep 18, 2025 at 01:43:46PM +0300, Tariq Toukan wrote:
> From: Gal Pressman <gal@nvidia.com>
> 
> Add a new Coccinelle script to identify places where PTR_ERR() is used
> in print functions and suggest using the %pe format specifier instead.
> 
> For printing error pointers (i.e., a pointer for which IS_ERR() is true)
> %pe will print a symbolic error name (e.g,. -EINVAL), opposed to the raw
> errno (e.g,. -22) produced by PTR_ERR().
> It also makes the code cleaner by saving a redundant call to PTR_ERR().
> 
> The script supports context, report, and org modes.
> 
> Example transformation:
>     printk("Error: %ld\n", PTR_ERR(ptr));  // Before
>     printk("Error: %pe\n", ptr);          // After
> 
> Signed-off-by: Gal Pressman <gal@nvidia.com>
> Reviewed-by: Alexei Lazar <alazar@nvidia.com>
> Signed-off-by: Tariq Toukan <tariqt@nvidia.com>

Thanks, having this check seems very nice to me.

Reviewed-by: Simon Horman <horms@kernel.org>