lkml.org 
[lkml]   [2019]   [Mar]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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
>
>

\
 
 \ /
  Last update: 2019-03-15 08:30    [W:0.027 / U:0.700 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site