Re: [Cocci] [PATCH v2] coccinelle: semantic code search for missingof_node_put

[<wen.yang99@zte.com.cn>]

[-- Attachment #1.1: Type: text/plain, Size: 1648 bytes --]

> > > +x = @p1\(of_find_all_nodes\|
> >
> > I would find this SmPL disjunction easier to read without the usage
> > of extra backslashes.
> >
> > +x =
> > +(of_…
> > +|of_…
> > +)@p1(...);
> 
> Did you actually test this?  I doubt that a position metavariable can be
> put on a ) of a disjunction.
> 
> > > +|
> > > +return x;
> > > +|
> > > +return of_fwnode_handle(x);
> >
> > Can a nested SmPL disjunction be helpful at such places?
> >
> > +|return
> > +(x
> > +|of_fwnode_handle(x)
> > +);
> 
> The original code is much more readable.  The internal representation will
> be the same.
> 
> > > +    when != v4l2_async_notifier_add_fwnode_subdev(<...x...>)
> >
> > Would the specification variant “<+... x ...+>” be relevant
> > for the parameter selection?
> 
> I'm indeed quite surprised that <...x...> would be accepted by the parser..

Hi julia,

Thank you for your comments.
We tested and found that both <...x...> and <+... x ...+> variants work fine.
We use <... x ...> instead of <+... x ...+> here to eliminate the following false positives:

./drivers/media/platform/qcom/camss/camss.c:504:1-7: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 479, but without a corresponding object release within this function.

465 static int camss_of_parse_ports(struct camss *camss)
466 {
...
479 remote = of_graph_get_remote_port_parent(node);
...
486 asd = v4l2_async_notifier_add_fwnode_subdev(
487 &camss->notifier, of_fwnode_handle(remote), ---> v4l2_async_notifier_add_fwnode_subdev will pass remote to camss->notifier.
488 sizeof(*csd));
...
504 return num_subdevs;

--
Thanks and regards,
Wen

[-- Attachment #2: Type: text/plain, Size: 136 bytes --]

_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci