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

[Julia Lawall]

> >> +@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?
>