lkml.org 
[lkml]   [2019]   [May]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH] coccinelle: semantic patch for missing of_node_put
From
Date
> 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

\
 
 \ /
  Last update: 2019-05-07 17:28    [W:0.030 / U:0.500 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site