Re: [cocci] [PATCH net-next 1/2] scripts/coccinelle: Find PTR_ERR() to %pe candidates

[Gal Pressman <gal@nvidia.com>]

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.