Messages in this thread | | | Date | Fri, 15 Mar 2019 08:29:49 +0100 (CET) | From | Julia Lawall <> | Subject | Re: [PATCH] coccinelle: semantic patch for missing of_node_put |
| |
On Fri, 15 Mar 2019, Wen Yang wrote:
> Looking for places where there is an of_node_put on some paths > but not on others. This SmPL checks that there is a put of the > same data elsewhere in the function, so perhaps that will > alleviate the concern about puts where they are not needed, > while still making it possible to find the ones that are needed. > > Suggested-by: Julia Lawall <Julia.Lawall@lip6.fr> > Signed-off-by: Wen Yang <wen.yang99@zte.com.cn> > Reviewed-by: Julia Lawall <Julia.Lawall@lip6.fr> > --- > scripts/coccinelle/free/of_node_put.cocci | 57 +++++++++++++++++++++++++++++++ > 1 file changed, 57 insertions(+) > create mode 100644 scripts/coccinelle/free/of_node_put.cocci > > diff --git a/scripts/coccinelle/free/of_node_put.cocci b/scripts/coccinelle/free/of_node_put.cocci > new file mode 100644 > index 0000000..6a29830 > --- /dev/null > +++ b/scripts/coccinelle/free/of_node_put.cocci > @@ -0,0 +1,57 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/// Find missing of_node_put > +/// > +// Confidence: Moderate > +// Comments: > +// Options: --no-includes --include-headers > + > +virtual report > +virtual org > + > +@r exists@ > +local idexpression struct device_node *x; > +identifier f; > +statement S,S1,S2; > +expression e,e1; > +position p1,p2; > +type T,T1; > +@@ > + > +( > +x = f@p1(...); > +... when != e = (T)x
I suggest the following:
( if (x) { ... of_node_put(x) ... } | > +if (x == NULL || ...) S1 > +else S2 > +... when != of_node_put(x) > + when != if (x) { ... of_node_put(x) ... } > + when != e1 = (T1)x > +( > +return x; > +| > +return@p2 ...; > +) )
If the first test is for success of the allocation and may lead to an of_node_put, then you can stop. Perhaps
if (x) { ... when forall of_node_put(x) ... }
there would be better, to check if there is always a put. This could also be done on the other if (x)
> +& > +x = f(...) > +... > +if (<+...x...+>) S > +... > +of_node_put(x);
There is actually an opportunity here for more reports. Perhaps we can assume that if the function calls of_node_put on anything, then it is needed on everything. So this could be of_node_put(...). But the downside of that is that the original x = f(...) may now let through things that are not reference counted. So you would want two rules, first this one where the function is f and there is a of_node_put on the result of the function, and another one where you consider a known set of functions, and then allow a subsequent of_node_put on anything.
It would take some care to ensure that there is only one report per call site.
julia
> +) > + > +@script:python depends on report@ > +p1 << r.p1; > +p2 << r.p2; > +@@ > + > +coccilib.report.print_report(p2[0], > + "ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line " > + + p1[0].line > + + ", but without a corresponding object release within this function.") > + > +@script:python depends on org@ > +p1 << r.p1; > +p2 << r.p2; > +@@ > + > +cocci.print_main("acquired a node pointer with refcount incremented", p1) > +cocci.print_secs("needed of_node_put", p2) > -- > 2.9.5 > >
| |