> The counter must be decremented after the last usage of a device node.
Thanks for your next try to improve the software situation also in this area. > We find these functions by using the following SmPL: Would it be nicer to use the word “script” also here? > <SmPL> > @initialize:ocaml@ > @@ How do you think about to describe the chosen algorithm in a way for contributors who might not be so familiar with this programming language? Will any information from previous discussions become relevant for a better commit description? > let relevant_str = "use of_node_put() on it when done" Will such a literal need further development and software documentation considerations? > let contains s1 s2 = > let re = Str.regexp_string s2 > in > try ignore (Str.search_forward re s1 0); true > with Not_found -> false > > let relevant_functions = ref [] > > let add_function f c = > if not (List.mem f !relevant_functions) > then > begin > let s = String.concat " " I find such a concatenation suspicious after the space character is used also for a string splitting before. Can this delimiter be omitted for the combination? > ( > (List.map String.lowercase_ascii > (List.filter > (function x -> > Str.string_match > (Str.regexp "[a-zA-Z_\\(\\)][-a-zA-Z0-9_\\(\\)]*$") > x 0) (Str.split (Str.regexp "[ .;\t\n]+") c)))) in > if contains s relevant_str I would prefer to use the string constant in the called function directly instead of passing it as another parameter. > then > Printf.printf "Found relevant function: %s\n" f; > relevant_functions := f :: !relevant_functions; > end I find your choice for an output format unclear at the moment. I imagine that the corresponding data processing of these function names will need fine-tuning. I am missing the software component for the conversion of this identifier list into a disjunction for the SmPL rule “r1”. > And this patch also looks for places where an of_node_put() Does a patch or a script perform an action? > call is on some paths but not on others. Let us look at mentioned implementation details. > +@initialize:python@ > +@@ > + > +seen = set() > + > +def add_if_not_present (p1, p2): It seems that you would like to use iteration functionality. https://github.com/coccinelle/coccinelle/blob/99e081e9b89d49301b7bd2c5e5aac88c66eaaa6a/docs/manual/cocci_syntax.tex#L1826 How will it matter here? > +def display_report(p1, p2): > + if add_if_not_present(p1[0].line, p2[0].line): > + 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.") > + > +def display_org(p1, p2): > + cocci.print_main("acquired a node pointer with refcount incremented", p1) > + cocci.print_secs("needed of_node_put", p2) Can it be helpful to specify SmPL dependencies for these functions according to the applied operation mode? > +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(...); Which sort criteria were applied for the generation of the shown function name list? > +if (x == NULL || ...) S > +... when != e = (T)x > + when != true x == NULL I wonder if this code exclusion specification is really required after a null pointer was checked before. > +| > +return x; > +| > +return of_fwnode_handle(x); Can a nested SmPL disjunction be helpful at such places? +|return +(x +|of_fwnode_handle(x) +); > + when != v4l2_async_notifier_add_fwnode_subdev(<...x...>) Would the specification variant “<+... x ...+>” be relevant for the parameter selection? > +& > +x = f(...) > +... > +if (<+...x...+>) S > +... > +of_node_put(x); You propose once more to use a SmPL conjunction in the rule “r2”. How does it fit to the previous exclusion specification “when != of_node_put(x)”? Regards, Markus