Messages in this thread | | | Subject | Re: [PATCH] coccinelle: semantic patch for missing of_node_put | From | Markus Elfring <> | Date | Tue, 7 May 2019 17:27:15 +0200 |
| |
> The call to of_parse_phandle()/of_find_node_by_name() ... returns a node > pointer with refcount incremented thus it must be explicitly decremented > after the last usage. > > This SmPL is also looking for places where there is an of_node_put on > some path but not on others.
I suggest to improve this commit description.
* Possible wording: There are functions which increment a reference counter for a device node. These functions belong to a programming interface for the management of information from device trees. The counter must be decremented after the last usage of a device node.
This SmPL script looks also for places where a of_node_put() call is on some paths but not on others.
* Will the word “patch” be replaced by “code search” in the commit subject because the operation modes “report” and “org” are supported here?
> +@initialize:python@ > +@@
Such a SmPL rule would apply to every possible operation mode. I have noticed then that the two Python variables from here will be needed only in two SmPL rules which depend on the mode “report”.
* Thus I would prefer to adjust the dependency specification accordingly.
* Please replace these variables by a separate function like the following. def display1(p1 ,p2): if add_if_not_present(p1[0].line, p2[0].line): coccilib.report.print_report(p2[0], "prefix" + p1[0].line + "suffix")
* Please move another bit of duplicate code to a separate function like the following. def display2(p1 ,p2): cocci.print_main("Choose info 1", p1) cocci.print_secs("Choose info 2", p2)
> +x = @p1\(of_find_compatible_node\|of_find_node_by_name\|of_parse_phandle\|
If you would like to insist to use such a SmPL disjunction, I would prefer an other code formatting here. How do you think about to put each function name on a separate line?
Can such a name list be ever automatically determined from an other information source? (Are there circumstances to consider under which the application of a detailed regular expression would become interesting for a SmPL constraint?)
Will it be influenced by any sort criteria?
> + when != of_node_put(x) … > + when != if (x) { ... of_node_put(x) ... }
I find the second when constraint specification unnecessary because the previous one should be sufficient to exclude such a function call.
Can the specification “when != \( of_node_put \| of_get_next_parent \) (x)” be useful?
> +return x; > +| > +return of_fwnode_handle(x);
Can it be nicer to merge this bit of code into another SmPL disjunction?
+return \( x \| of_fwnode_handle(x) \);
Regards, Markus
| |