* [Cocci] [PATCH v2] coccinelle: semantic code search for missing of_node_put @ 2019-06-28 2:58 Wen Yang 2019-06-28 9:38 ` Markus Elfring 0 siblings, 1 reply; 17+ messages in thread From: Wen Yang @ 2019-06-28 2:58 UTC (permalink / raw) To: linux-kernel Cc: wang.yi59, Michal Marek, Nicolas Palix, cocci, Markus Elfring, Wen Yang 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. We find these functions by using the following SmPL: <SmPL> @initialize:ocaml@ @@ let relevant_str = "use of_node_put() on it when done" 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 " " ( (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 then Printf.printf "Found relevant function: %s\n" f; relevant_functions := f :: !relevant_functions; end @r@ identifier fn; comments c; type T = struct device_node *; @@ T@c fn(...) { ... } @script:ocaml@ f << r.fn; c << r.c; @@ let (cb,cm,ca) = List.hd c in let c = String.concat " " cb in add_function f c </SmPL> And this patch also looks for places where an of_node_put() call is on some paths but not on others. Suggested-by: Julia Lawall <Julia.Lawall@lip6.fr> Signed-off-by: Wen Yang <wen.yang99@zte.com.cn> Cc: Julia Lawall <Julia.Lawall@lip6.fr> Cc: Gilles Muller <Gilles.Muller@lip6.fr> Cc: Nicolas Palix <nicolas.palix@imag.fr> Cc: Michal Marek <michal.lkml@markovi.net> Cc: Masahiro Yamada <yamada.masahiro@socionext.com> Cc: Markus Elfring <Markus.Elfring@web.de> Cc: cocci@systeme.lip6.fr --- v2: improve the commit description and delete duplicate code. scripts/coccinelle/free/of_node_put.cocci | 143 ++++++++++++++++++++++++++++++ 1 file changed, 143 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..81a9f54 --- /dev/null +++ b/scripts/coccinelle/free/of_node_put.cocci @@ -0,0 +1,143 @@ +// SPDX-License-Identifier: GPL-2.0 +/// Find missing of_node_put +/// +// Confidence: Moderate +// Copyright: (C) 2018-2019 Wen Yang, ZTE. +// Comments: +// Options: --no-includes --include-headers + +virtual report +virtual org + +@initialize:python@ +@@ + +seen = set() + +def add_if_not_present (p1, p2): + if (p1, p2) not in seen: + seen.add((p1, p2)) + return True + return False + +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) + +@r1 exists@ +local idexpression struct device_node *x; +expression e, e1; +position p1, p2; +statement S; +type T; +@@ + +x = @p1\(of_find_all_nodes\| + of_get_cpu_node\| + of_get_parent\| + of_get_next_parent\| + of_get_next_child\| + of_get_next_cpu_node\| + of_get_compatible_child\| + of_get_child_by_name\| + of_find_node_opts_by_path\| + of_find_node_by_name\| + of_find_node_by_type\| + of_find_compatible_node\| + of_find_node_with_property\| + of_find_matching_node_and_match\| + of_find_node_by_phandle\| + of_parse_phandle\| + of_find_next_cache_node\| + of_get_next_available_child\)(...); +... +if (x == NULL || ...) S +... when != e = (T)x + when != true x == NULL + when != of_node_put(x) + when != of_get_next_parent(x) + when != of_find_matching_node(x, ...) + when != if (x) { ... return x; } + when != v4l2_async_notifier_add_fwnode_subdev(<...x...>) + when != e1 = of_fwnode_handle(x) +( +if (x) { ... when forall + of_node_put(x) ... } +| +return x; +| +return of_fwnode_handle(x); +| +return@p2 ...; +) + +@script:python depends on report && r1@ +p1 << r1.p1; +p2 << r1.p2; +@@ + +display_report(p1, p2) + +@script:python depends on org && r1@ +p1 << r1.p1; +p2 << r1.p2; +@@ + +display_org(p1, p2) + +@r2 exists@ +local idexpression struct device_node *x; +expression e, e1; +position p1, p2; +identifier f; +statement S; +type T; +@@ + +( +x = f@p1(...); +... when != e = (T)x + when != true x == NULL + when != of_node_put(x) + when != of_get_next_parent(x) + when != of_find_matching_node(x, ...) + when != if (x) { ... return x; } + when != v4l2_async_notifier_add_fwnode_subdev(<...x...>) + when != e1 = of_fwnode_handle(x) +( +if (x) { ... when forall + of_node_put(x) ... } +| +return x; +| +return of_fwnode_handle(x); +| +return@p2 ...; +) +& +x = f(...) +... +if (<+...x...+>) S +... +of_node_put(x); +) +@script:python depends on report && r2@ +p1 << r2.p1; +p2 << r2.p2; +@@ + +display_report(p1, p2) + +@script:python depends on org && r2@ +p1 << r2.p1; +p2 << r2.p2; +@@ + +display_org(p1, p2) -- 2.9.5 _______________________________________________ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [Cocci] [PATCH v2] coccinelle: semantic code search for missing of_node_put 2019-06-28 2:58 [Cocci] [PATCH v2] coccinelle: semantic code search for missing of_node_put Wen Yang @ 2019-06-28 9:38 ` Markus Elfring 2019-06-28 11:07 ` Julia Lawall ` (2 more replies) 0 siblings, 3 replies; 17+ messages in thread From: Markus Elfring @ 2019-06-28 9:38 UTC (permalink / raw) To: Wen Yang, linux-kernel; +Cc: Yi Wang, Michal Marek, Nicolas Palix, cocci > 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 _______________________________________________ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Cocci] [PATCH v2] coccinelle: semantic code search for missing of_node_put 2019-06-28 9:38 ` Markus Elfring @ 2019-06-28 11:07 ` Julia Lawall 2019-06-28 14:16 ` [Cocci] [v2] " Markus Elfring ` (2 more replies) 2019-06-29 7:40 ` Markus Elfring 2019-07-04 2:41 ` [Cocci] 答复: Re: [PATCH v2] coccinelle: semantic code search for missingof_node_put wen.yang99 2 siblings, 3 replies; 17+ messages in thread From: Julia Lawall @ 2019-06-28 11:07 UTC (permalink / raw) To: Markus Elfring Cc: Yi Wang, Michal Marek, Nicolas Palix, linux-kernel, cocci, Wen Yang [-- Attachment #1: Type: text/plain, Size: 788 bytes --] > > +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(...); Did you actually test this? I doubt that a position metavariable can be put on a ) of a disjunction. > > +| > > +return x; > > +| > > +return of_fwnode_handle(x); > > Can a nested SmPL disjunction be helpful at such places? > > +|return > +(x > +|of_fwnode_handle(x) > +); The original code is much more readable. The internal representation will be the same. > > + when != v4l2_async_notifier_add_fwnode_subdev(<...x...>) > > Would the specification variant “<+... x ...+>” be relevant > for the parameter selection? I'm indeed quite surprised that <...x...> would be accepted by the parser. julia [-- Attachment #2: Type: text/plain, Size: 136 bytes --] _______________________________________________ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Cocci] [v2] coccinelle: semantic code search for missing of_node_put 2019-06-28 11:07 ` Julia Lawall @ 2019-06-28 14:16 ` Markus Elfring 2019-06-28 14:16 ` Markus Elfring 2019-07-04 3:03 ` [Cocci] [PATCH v2] coccinelle: semantic code search for missingof_node_put wen.yang99 2 siblings, 0 replies; 17+ messages in thread From: Markus Elfring @ 2019-06-28 14:16 UTC (permalink / raw) To: Julia Lawall, cocci Cc: Yi Wang, Michal Marek, Nicolas Palix, linux-kernel, Wen Yang >> +x = >> +(of_… >> +|of_… >> +)@p1(...); > > Did you actually test this? I doubt that a position metavariable can be > put on a ) of a disjunction. Would you ever like to support this possibility? >> +|return >> +(x >> +|of_fwnode_handle(x) >> +); > > The original code is much more readable. We have got different views around such specification variants. > The internal representation will be the same. I imagine that the Coccinelle software might evolve into additional directions. Regards, Markus _______________________________________________ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Cocci] [v2] coccinelle: semantic code search for missing of_node_put 2019-06-28 11:07 ` Julia Lawall 2019-06-28 14:16 ` [Cocci] [v2] " Markus Elfring @ 2019-06-28 14:16 ` Markus Elfring 2019-07-04 3:03 ` [Cocci] [PATCH v2] coccinelle: semantic code search for missingof_node_put wen.yang99 2 siblings, 0 replies; 17+ messages in thread From: Markus Elfring @ 2019-06-28 14:16 UTC (permalink / raw) To: Julia Lawall, cocci Cc: Yi Wang, Michal Marek, Nicolas Palix, linux-kernel, Wen Yang >> +x = >> +(of_… >> +|of_… >> +)@p1(...); > > Did you actually test this? I doubt that a position metavariable can be > put on a ) of a disjunction. Would you ever like to support this possibility? >> +|return >> +(x >> +|of_fwnode_handle(x) >> +); > > The original code is much more readable. We have got different views around such specification variants. > The internal representation will be the same. I imagine that the Coccinelle software might evolve into additional directions. Regards, Markus _______________________________________________ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Cocci] [PATCH v2] coccinelle: semantic code search for missingof_node_put 2019-06-28 11:07 ` Julia Lawall 2019-06-28 14:16 ` [Cocci] [v2] " Markus Elfring 2019-06-28 14:16 ` Markus Elfring @ 2019-07-04 3:03 ` wen.yang99 2019-07-04 6:28 ` [Cocci] [v2] coccinelle: semantic code search for missing of_node_put Markus Elfring 2019-07-05 5:29 ` [Cocci] [PATCH v2] coccinelle: semantic code search for missingof_node_put Julia Lawall 2 siblings, 2 replies; 17+ messages in thread From: wen.yang99 @ 2019-07-04 3:03 UTC (permalink / raw) To: julia.lawall Cc: wang.yi59, michal.lkml, nicolas.palix, linux-kernel, Markus.Elfring, cocci [-- Attachment #1.1: Type: text/plain, Size: 1648 bytes --] > > > +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(...); > > Did you actually test this? I doubt that a position metavariable can be > put on a ) of a disjunction. > > > > +| > > > +return x; > > > +| > > > +return of_fwnode_handle(x); > > > > Can a nested SmPL disjunction be helpful at such places? > > > > +|return > > +(x > > +|of_fwnode_handle(x) > > +); > > The original code is much more readable. The internal representation will > be the same. > > > > + when != v4l2_async_notifier_add_fwnode_subdev(<...x...>) > > > > Would the specification variant “<+... x ...+>” be relevant > > for the parameter selection? > > I'm indeed quite surprised that <...x...> would be accepted by the parser.. Hi julia, Thank you for your comments. We tested and found that both <...x...> and <+... x ...+> variants work fine. We use <... x ...> instead of <+... x ...+> here to eliminate the following false positives: ./drivers/media/platform/qcom/camss/camss.c:504:1-7: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 479, but without a corresponding object release within this function. 465 static int camss_of_parse_ports(struct camss *camss) 466 { ... 479 remote = of_graph_get_remote_port_parent(node); ... 486 asd = v4l2_async_notifier_add_fwnode_subdev( 487 &camss->notifier, of_fwnode_handle(remote), ---> v4l2_async_notifier_add_fwnode_subdev will pass remote to camss->notifier. 488 sizeof(*csd)); ... 504 return num_subdevs; -- Thanks and regards, Wen [-- Attachment #2: Type: text/plain, Size: 136 bytes --] _______________________________________________ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Cocci] [v2] coccinelle: semantic code search for missing of_node_put 2019-07-04 3:03 ` [Cocci] [PATCH v2] coccinelle: semantic code search for missingof_node_put wen.yang99 @ 2019-07-04 6:28 ` Markus Elfring 2019-07-05 5:29 ` [Cocci] [PATCH v2] coccinelle: semantic code search for missingof_node_put Julia Lawall 1 sibling, 0 replies; 17+ messages in thread From: Markus Elfring @ 2019-07-04 6:28 UTC (permalink / raw) To: Wen Yang, Julia Lawall Cc: Yi Wang, Michal Marek, Nicolas Palix, linux-kernel, cocci > We tested and found that both <...x...> and <+... x ...+> variants work fine. Is the difference in the functionality from this SmPL construct clear already? > We use <... x ...> instead of <+... x ...+> here to eliminate the following false positives: Do we stumble on another software design challenge? For which function parameter will the specified variable be required finally? > 486 asd = v4l2_async_notifier_add_fwnode_subdev( > 487 &camss->notifier, of_fwnode_handle(remote), ---> v4l2_async_notifier_add_fwnode_subdev will pass remote to camss->notifier. > 488 sizeof(*csd)); Should any more special cases be taken better into account? Regards, Markus _______________________________________________ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Cocci] [PATCH v2] coccinelle: semantic code search for missingof_node_put 2019-07-04 3:03 ` [Cocci] [PATCH v2] coccinelle: semantic code search for missingof_node_put wen.yang99 2019-07-04 6:28 ` [Cocci] [v2] coccinelle: semantic code search for missing of_node_put Markus Elfring @ 2019-07-05 5:29 ` Julia Lawall 2019-07-05 5:57 ` [Cocci] [PATCH v2] coccinelle: semantic code search formissingof_node_put wen.yang99 1 sibling, 1 reply; 17+ messages in thread From: Julia Lawall @ 2019-07-05 5:29 UTC (permalink / raw) To: wen.yang99 Cc: wang.yi59, michal.lkml, nicolas.palix, linux-kernel, Markus.Elfring, cocci [-- Attachment #1: Type: text/plain, Size: 2124 bytes --] On Thu, 4 Jul 2019, wen.yang99@zte.com.cn wrote: > > > > +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(...); > > > > Did you actually test this? I doubt that a position metavariable can be > > put on a ) of a disjunction. > > > > > > +| > > > > +return x; > > > > +| > > > > +return of_fwnode_handle(x); > > > > > > Can a nested SmPL disjunction be helpful at such places? > > > > > > +|return > > > +(x > > > +|of_fwnode_handle(x) > > > +); > > > > The original code is much more readable. The internal representation will > > be the same. > > > > > > + when != v4l2_async_notifier_add_fwnode_subdev(<...x...>) > > > > > > Would the specification variant “<+... x ...+>” be relevant > > > for the parameter selection? > > > > I'm indeed quite surprised that <...x...> would be accepted by the parser.. > > Hi julia, > > Thank you for your comments. > We tested and found that both <...x...> and <+... x ...+> variants work fine. > We use <... x ...> instead of <+... x ...+> here to eliminate the following false positives: > > ./drivers/media/platform/qcom/camss/camss.c:504:1-7: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 479, but without a corresponding object release within this function. > > 465 static int camss_of_parse_ports(struct camss *camss) > 466 { > ... > 479 remote = of_graph_get_remote_port_parent(node); > ... > 486 asd = v4l2_async_notifier_add_fwnode_subdev( > 487 &camss->notifier, of_fwnode_handle(remote), ---> v4l2_async_notifier_add_fwnode_subdev will pass remote to camss->notifier. > 488 sizeof(*csd)); > ... > 504 return num_subdevs; I suspect that what is happening is that there is a runtime error, but that error is caught somewhere and you don't see it. Could you send me again the entire semantic patch so I can check on this? I think that what you want is: when != v4l2_async_notifier_add_fwnode_subdev(...,<+...x...+>,...) ie x occurring somewhere within some argument. julia [-- Attachment #2: Type: text/plain, Size: 136 bytes --] _______________________________________________ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Cocci] [PATCH v2] coccinelle: semantic code search formissingof_node_put 2019-07-05 5:29 ` [Cocci] [PATCH v2] coccinelle: semantic code search for missingof_node_put Julia Lawall @ 2019-07-05 5:57 ` wen.yang99 2019-07-05 6:17 ` Julia Lawall 0 siblings, 1 reply; 17+ messages in thread From: wen.yang99 @ 2019-07-05 5:57 UTC (permalink / raw) To: julia.lawall Cc: wang.yi59, michal.lkml, nicolas.palix, linux-kernel, Markus.Elfring, cocci [-- Attachment #1.1: Type: text/plain, Size: 5877 bytes --] > > > > > +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(...); > > > > > > Did you actually test this? I doubt that a position metavariable can be > > > put on a ) of a disjunction. > > > > > > > > +| > > > > > +return x; > > > > > +| > > > > > +return of_fwnode_handle(x); > > > > > > > > Can a nested SmPL disjunction be helpful at such places? > > > > > > > > +|return > > > > +(x > > > > +|of_fwnode_handle(x) > > > > +); > > > > > > The original code is much more readable. The internal representation will > > > be the same. > > > > > > > > + when != v4l2_async_notifier_add_fwnode_subdev(<...x...>) > > > > > > > > Would the specification variant “<+... x ...+>” be relevant > > > > for the parameter selection? > > > > > > I'm indeed quite surprised that <...x...> would be accepted by the parser.. > > > > Hi julia, > > > > Thank you for your comments. > > We tested and found that both <...x...> and <+... x ...+> variants work fine. > > We use <... x ...> instead of <+... x ...+> here to eliminate the following false positives: > > > > ./drivers/media/platform/qcom/camss/camss.c:504:1-7: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 479, but without a corresponding object release within this function. > > > > 465 static int camss_of_parse_ports(struct camss *camss) > > 466 { > > ... > > 479 remote = of_graph_get_remote_port_parent(node); > > ... > > 486 asd = v4l2_async_notifier_add_fwnode_subdev( > > 487 &camss->notifier, of_fwnode_handle(remote), ---> v4l2_async_notifier_add_fwnode_subdev will pass remote to camss->notifier. > > 488 sizeof(*csd)); > > ... > > 504 return num_subdevs; > > I suspect that what is happening is that there is a runtime error, but > that error is caught somewhere and you don't see it. Thanks. You are right, there is indeed a runtime error. Since make coccicheck adds the "-very-quiet" parameter by default, we didn't find it. $ spatch --sp-file of_node_put.cocci -D report drivers/media/platform/am437x/am437x-vpfe.c init_defs_builtins: /usr/local/bin/../lib/coccinelle/standard.h HANDLING: drivers/media/platform/am437x/am437x-vpfe.c exn while in timeout_function only handling multi and no when code in a nest expr > Could you send me again the entire semantic patch so I can check on this? > Thanks. The entire SmPL is as follows: $ cat of_node_put.cocci // SPDX-License-Identifier: GPL-2.0 /// Find missing of_node_put /// // Confidence: Moderate // Copyright: (C) 2018-2019 Wen Yang, ZTE. // Comments: // Options: --no-includes --include-headers virtual report virtual org @initialize:python@ @@ seen = set() def add_if_not_present (p1, p2): if (p1, p2) not in seen: seen.add((p1, p2)) return True return False 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) @r1 exists@ local idexpression struct device_node *x; expression e, e1; position p1, p2; statement S; type T; @@ x = @p1\(of_find_all_nodes\| of_get_cpu_node\| of_get_parent\| of_get_next_parent\| of_get_next_child\| of_get_next_cpu_node\| of_get_compatible_child\| of_get_child_by_name\| of_find_node_opts_by_path\| of_find_node_by_name\| of_find_node_by_type\| of_find_compatible_node\| of_find_node_with_property\| of_find_matching_node_and_match\| of_find_node_by_phandle\| of_parse_phandle\| of_find_next_cache_node\| of_get_next_available_child\)(...); ... if (x == NULL || ...) S ... when != e = (T)x when != true x == NULL when != of_node_put(x) when != of_get_next_parent(x) when != of_find_matching_node(x, ...) when != if (x) { ... return x; } when != v4l2_async_notifier_add_fwnode_subdev(<...x...>) when != e1 = of_fwnode_handle(x) ( if (x) { ... when forall of_node_put(x) ... } | return x; | return of_fwnode_handle(x); | return@p2 ...; ) @script:python depends on report && r1@ p1 << r1.p1; p2 << r1.p2; @@ display_report(p1, p2) @script:python depends on org && r1@ p1 << r1.p1; p2 << r1.p2; @@ display_org(p1, p2) @r2 exists@ local idexpression struct device_node *x; expression e, e1; position p1, p2; identifier f; statement S; type T; @@ ( x = f@p1(...); ... when != e = (T)x when != true x == NULL when != of_node_put(x) when != of_get_next_parent(x) when != of_find_matching_node(x, ...) when != if (x) { ... return x; } when != v4l2_async_notifier_add_fwnode_subdev(<...x...>) when != e1 = of_fwnode_handle(x) ( if (x) { ... when forall of_node_put(x) ... } | return x; | return of_fwnode_handle(x); | return@p2 ...; ) & x = f(...) ... if (<+...x...+>) S ... of_node_put(x); ) @script:python depends on report && r2@ p1 << r2.p1; p2 << r2.p2; @@ display_report(p1, p2) @script:python depends on org && r2@ p1 << r2.p1; p2 << r2.p2; @@ display_org(p1, p2) > I think that what you want is: > > when != v4l2_async_notifier_add_fwnode_subdev(...,<+...x...+>,...) > > ie x occurring somewhere within some argument. Thank you very much for your suggestion. Applying it will solve this problem, thank you. -- Thanks and regards, Wen [-- Attachment #2: Type: text/plain, Size: 136 bytes --] _______________________________________________ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Cocci] [PATCH v2] coccinelle: semantic code search formissingof_node_put 2019-07-05 5:57 ` [Cocci] [PATCH v2] coccinelle: semantic code search formissingof_node_put wen.yang99 @ 2019-07-05 6:17 ` Julia Lawall 2019-07-05 6:45 ` [Cocci] [v2] coccinelle: semantic code search for missing of_node_put Markus Elfring 0 siblings, 1 reply; 17+ messages in thread From: Julia Lawall @ 2019-07-05 6:17 UTC (permalink / raw) To: wen.yang99 Cc: wang.yi59, michal.lkml, nicolas.palix, linux-kernel, Markus.Elfring, cocci [-- Attachment #1: Type: text/plain, Size: 6974 bytes --] On Fri, 5 Jul 2019, wen.yang99@zte.com.cn wrote: > > > > > > +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(...); > > > > > > > > Did you actually test this? I doubt that a position metavariable can be > > > > put on a ) of a disjunction. > > > > > > > > > > +| > > > > > > +return x; > > > > > > +| > > > > > > +return of_fwnode_handle(x); > > > > > > > > > > Can a nested SmPL disjunction be helpful at such places? > > > > > > > > > > +|return > > > > > +(x > > > > > +|of_fwnode_handle(x) > > > > > +); > > > > > > > > The original code is much more readable. The internal representation will > > > > be the same. > > > > > > > > > > + when != v4l2_async_notifier_add_fwnode_subdev(<...x...>) > > > > > > > > > > Would the specification variant “<+... x ...+>” be relevant > > > > > for the parameter selection? > > > > > > > > I'm indeed quite surprised that <...x...> would be accepted by the parser.. > > > > > > Hi julia, > > > > > > Thank you for your comments. > > > We tested and found that both <...x...> and <+... x ...+> variants work fine. > > > We use <... x ...> instead of <+... x ...+> here to eliminate the following false positives: > > > > > > ./drivers/media/platform/qcom/camss/camss.c:504:1-7: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 479, but without a corresponding object release within this function. > > > > > > 465 static int camss_of_parse_ports(struct camss *camss) > > > 466 { > > > ... > > > 479 remote = of_graph_get_remote_port_parent(node); > > > ... > > > 486 asd = v4l2_async_notifier_add_fwnode_subdev( > > > 487 &camss->notifier, of_fwnode_handle(remote), ---> v4l2_async_notifier_add_fwnode_subdev will pass remote to camss->notifier. > > > 488 sizeof(*csd)); > > > ... > > > 504 return num_subdevs; > > > > I suspect that what is happening is that there is a runtime error, but > > that error is caught somewhere and you don't see it. > > Thanks. > You are right, there is indeed a runtime error. > Since make coccicheck adds the "-very-quiet" parameter by default, we didn't find it. > > $ spatch --sp-file of_node_put.cocci -D report drivers/media/platform/am437x/am437x-vpfe.c > init_defs_builtins: /usr/local/bin/../lib/coccinelle/standard.h > HANDLING: drivers/media/platform/am437x/am437x-vpfe.c > exn while in timeout_function > only handling multi and no when code in a nest expr > > > Could you send me again the entire semantic patch so I can check on this? > > > > Thanks. > The entire SmPL is as follows: > > $ cat of_node_put.cocci > // SPDX-License-Identifier: GPL-2.0 > /// Find missing of_node_put > /// > // Confidence: Moderate > // Copyright: (C) 2018-2019 Wen Yang, ZTE. > // Comments: > // Options: --no-includes --include-headers > > virtual report > virtual org > > @initialize:python@ > @@ > > seen = set() > > def add_if_not_present (p1, p2): > if (p1, p2) not in seen: > seen.add((p1, p2)) > return True > return False Did you need this? Normally a script rule is run only once for each set of bindings for the inherited variables. I guess that multiple p1s could lead to the same p2, and you only want to report on one of them? This set is going to be global to the whole kernel, or at least to all of the files considered by a given thread, if you use -j. To clean it up on each file, you can make another python at the end that depends on r1 or r2 and depends on report. This rule can clear seen. Otherwise, it looks fine. julia > > 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) > > @r1 exists@ > local idexpression struct device_node *x; > expression e, e1; > position p1, p2; > statement S; > type T; > @@ > > x = @p1\(of_find_all_nodes\| > of_get_cpu_node\| > of_get_parent\| > of_get_next_parent\| > of_get_next_child\| > of_get_next_cpu_node\| > of_get_compatible_child\| > of_get_child_by_name\| > of_find_node_opts_by_path\| > of_find_node_by_name\| > of_find_node_by_type\| > of_find_compatible_node\| > of_find_node_with_property\| > of_find_matching_node_and_match\| > of_find_node_by_phandle\| > of_parse_phandle\| > of_find_next_cache_node\| > of_get_next_available_child\)(...); > ... > if (x == NULL || ...) S > ... when != e = (T)x > when != true x == NULL > when != of_node_put(x) > when != of_get_next_parent(x) > when != of_find_matching_node(x, ...) > when != if (x) { ... return x; } > when != v4l2_async_notifier_add_fwnode_subdev(<...x...>) > when != e1 = of_fwnode_handle(x) > ( > if (x) { ... when forall > of_node_put(x) ... } > | > return x; > | > return of_fwnode_handle(x); > | > return@p2 ...; > ) > > @script:python depends on report && r1@ No need to depend on r1. That is guaranteed by the inheritance on the metavariables below. > p1 << r1.p1; > p2 << r1.p2; > @@ > > display_report(p1, p2) > > @script:python depends on org && r1@ > p1 << r1.p1; > p2 << r1.p2; > @@ > > display_org(p1, p2) > > @r2 exists@ > local idexpression struct device_node *x; > expression e, e1; > position p1, p2; > identifier f; > statement S; > type T; > @@ > > ( > x = f@p1(...); > ... when != e = (T)x > when != true x == NULL > when != of_node_put(x) > when != of_get_next_parent(x) > when != of_find_matching_node(x, ...) > when != if (x) { ... return x; } > when != v4l2_async_notifier_add_fwnode_subdev(<...x...>) > when != e1 = of_fwnode_handle(x) > ( > if (x) { ... when forall > of_node_put(x) ... } > | > return x; > | > return of_fwnode_handle(x); > | > return@p2 ...; > ) > & > x = f(...) > ... > if (<+...x...+>) S > ... > of_node_put(x); > ) > @script:python depends on report && r2@ > p1 << r2.p1; > p2 << r2.p2; > @@ > > display_report(p1, p2) > > @script:python depends on org && r2@ > p1 << r2.p1; > p2 << r2.p2; > @@ > > display_org(p1, p2) > > > I think that what you want is: > > > > when != v4l2_async_notifier_add_fwnode_subdev(...,<+...x...+>,...) > > > > ie x occurring somewhere within some argument. > > Thank you very much for your suggestion. > Applying it will solve this problem, thank you. > > -- > Thanks and regards, > Wen [-- Attachment #2: Type: text/plain, Size: 136 bytes --] _______________________________________________ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Cocci] [v2] coccinelle: semantic code search for missing of_node_put 2019-07-05 6:17 ` Julia Lawall @ 2019-07-05 6:45 ` Markus Elfring 0 siblings, 0 replies; 17+ messages in thread From: Markus Elfring @ 2019-07-05 6:45 UTC (permalink / raw) To: Julia Lawall, Wen Yang Cc: Yi Wang, Michal Marek, Nicolas Palix, linux-kernel, cocci >> @script:python depends on report && r1@ > > No need to depend on r1. That is guaranteed by the inheritance on the > metavariables below. Will this information become more helpful for the completion of the corresponding software documentation? Regards, Markus _______________________________________________ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Cocci] [v2] coccinelle: semantic code search for missing of_node_put 2019-06-28 9:38 ` Markus Elfring 2019-06-28 11:07 ` Julia Lawall @ 2019-06-29 7:40 ` Markus Elfring 2019-06-29 7:49 ` Julia Lawall 2019-07-04 2:41 ` [Cocci] 答复: Re: [PATCH v2] coccinelle: semantic code search for missingof_node_put wen.yang99 2 siblings, 1 reply; 17+ messages in thread From: Markus Elfring @ 2019-06-29 7:40 UTC (permalink / raw) To: Wen Yang, linux-kernel, cocci; +Cc: Yi Wang, Michal Marek, Nicolas Palix >> +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. I would like to add another view for this implementation detail. The when constraint can express a software desire which can be reasonable to some degree. You would like to be sure that a null pointer will not occur after a corresponding check succeeded. * But I feel unsure about the circumstances under which the Coccinelle software can determine this aspect actually. * I find that it can eventually make sense only after the content of the local variable (which is identified by “x”) was modified. Thus I would find the exclusion of assignments more useful at this place. Regards, Markus _______________________________________________ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Cocci] [v2] coccinelle: semantic code search for missing of_node_put 2019-06-29 7:40 ` Markus Elfring @ 2019-06-29 7:49 ` Julia Lawall 2019-06-29 8:35 ` Markus Elfring 2019-06-29 19:30 ` [Cocci] [v2] Coccinelle: Testing SmPL constraints Markus Elfring 0 siblings, 2 replies; 17+ messages in thread From: Julia Lawall @ 2019-06-29 7:49 UTC (permalink / raw) To: Markus Elfring Cc: Yi Wang, Michal Marek, Nicolas Palix, linux-kernel, Wen Yang, cocci [-- Attachment #1: Type: text/plain, Size: 1088 bytes --] On Sat, 29 Jun 2019, Markus Elfring wrote: > >> +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. > > I would like to add another view for this implementation detail. > > The when constraint can express a software desire which can be reasonable > to some degree. You would like to be sure that a null pointer will not occur > after a corresponding check succeeded. He wants to be sure that the true branch through a NULL pointer check is not taken. > * But I feel unsure about the circumstances under which the Coccinelle software > can determine this aspect actually. > > * I find that it can eventually make sense only after the content of > the local variable (which is identified by “x”) was modified. > Thus I would find the exclusion of assignments more useful at this place. I assume that it was added because it was found to be useful. Please actually try things out before declaring them to be useless. julia [-- Attachment #2: Type: text/plain, Size: 136 bytes --] _______________________________________________ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Cocci] [v2] coccinelle: semantic code search for missing of_node_put 2019-06-29 7:49 ` Julia Lawall @ 2019-06-29 8:35 ` Markus Elfring 2019-06-29 19:30 ` [Cocci] [v2] Coccinelle: Testing SmPL constraints Markus Elfring 1 sibling, 0 replies; 17+ messages in thread From: Markus Elfring @ 2019-06-29 8:35 UTC (permalink / raw) To: Julia Lawall, cocci Cc: Yi Wang, Michal Marek, Nicolas Palix, linux-kernel, Wen Yang >>>> +if (x == NULL || ...) S >>>> +... when != e = (T)x >>>> + when != true x == NULL … > I assume that it was added because it was found to be useful. We can get different software development opinions also on this implementation detail. > Please actually try things out before declaring them to be useless. Further clarification of desirable software behaviour will help. I dare to express doubts around the SmPL functionality “when != true x == NULL”. Would any more contributors like to share additional insights for the safer application of the semantic patch language? Is a reassignment of such local variable an usual precondition for the discussed programming concern? Regards, Markus _______________________________________________ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Cocci] [v2] Coccinelle: Testing SmPL constraints 2019-06-29 7:49 ` Julia Lawall 2019-06-29 8:35 ` Markus Elfring @ 2019-06-29 19:30 ` Markus Elfring 1 sibling, 0 replies; 17+ messages in thread From: Markus Elfring @ 2019-06-29 19:30 UTC (permalink / raw) To: Julia Lawall, cocci Cc: Yi Wang, Michal Marek, Nicolas Palix, linux-kernel, Wen Yang > Please actually try things out before declaring them to be useless. This feedback provides also another opportunity for collateral evolution in some directions. I am curious on how involved uncertainty can be fixed around possibly different interpretation for provided software functionality. The SmPL construct “... when …” is mentioned in an area (of the section “Basic transformations” in the software documentation) which is introduced with the wording “The grammar for the minus or plus slice of a transformation is as follows:”. https://github.com/coccinelle/coccinelle/blob/c6d7554edf7c4654aeae4d33c3f040e300682f23/docs/manual/cocci_syntax.tex#L1033 I got the impression that the corresponding meaning is not explained in this information source so far. The published example “Reference counter: the of_xxx API” can be interesting then to some degree for the explanation of the discussed development efforts. https://github.com/coccinelle/coccinelle/blob/175de16bc7e535b6a89a62b81a673b0d0cd7075c/docs/manual/examples.tex#L320 If the available application documentation is still too limited (and incomplete because it is also work in progress), it is probably usual that SmPL code occasionally tries to express expectations which are not covered by an evolving software implementation. How would you like to improve the situation further? * Is it certain that a search is performed only for the source code “x == NULL” (and corresponding isomorphisms) by the SmPL constraint “when != true” (after a successful null pointer check was detected in this use case)? * Would you like to test any functionality which should work in different ways than you might see from the original OCaml source code? https://github.com/coccinelle/coccinelle/issues/134 Regards, Markus _______________________________________________ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci ^ permalink raw reply [flat|nested] 17+ messages in thread
* [Cocci] 答复: Re: [PATCH v2] coccinelle: semantic code search for missingof_node_put 2019-06-28 9:38 ` Markus Elfring 2019-06-28 11:07 ` Julia Lawall 2019-06-29 7:40 ` Markus Elfring @ 2019-07-04 2:41 ` wen.yang99 2019-07-04 5:40 ` [Cocci] [v2] coccinelle: semantic code search for missing of_node_put Markus Elfring 2 siblings, 1 reply; 17+ messages in thread From: wen.yang99 @ 2019-07-04 2:41 UTC (permalink / raw) To: Markus.Elfring; +Cc: wang.yi59, michal.lkml, nicolas.palix, linux-kernel, cocci [-- Attachment #1.1: Type: text/plain, Size: 5980 bytes --] > > 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? > OK, we will replace "SmPL" with "scrilt" later. > > > <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? > We will also provide an example written in Python later. > > > 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? > We first divide the comment by word, then use spaces to concatenate them into a string, and finally find the substring (use of_node_put() on it when done) in this string. > > > ( > > (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. Thanks. Here the relevant_str is a string constant. > > 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”. Thanks. We first use this script to find out all the function names to be processed, and then copy these function names into r1. > > And this patch also looks for places where an of_node_put() > > Does a patch or a script perform an action? > OK, Thanks. We'll fix it soon. > > 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? As julia pointed out, your current writing is not compiled. > > +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. Thanks. Our previous version used the "when any" clause, so we need "when != true x == NULL". We can delete this code exclusion specification for this version. We will fix it later. > > +| > > +return x; > > +| > > +return of_fwnode_handle(x); > > Can a nested SmPL disjunction be helpful at such places? > > +|return > +(x > +|of_fwnode_handle(x) > +); We are more in agreement with julia's comments: The original code is much more readable. The internal representation will be the same. > > + when != v4l2_async_notifier_add_fwnode_subdev(<...x...>) > > Would the specification variant “<+... x ...+>” be relevant > for the parameter selection? Thanks. We use <... x ...> instead of <+... x ...+> here to eliminate the following false positives: ./drivers/media/platform/qcom/camss/camss.c:504:1-7: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 479, but without a corresponding object release within this function. 465 static int camss_of_parse_ports(struct camss *camss) 466 { ... 479 remote = of_graph_get_remote_port_parent(node); ... 486 asd = v4l2_async_notifier_add_fwnode_subdev( 487 &camss->notifier, of_fwnode_handle(remote), ---> v4l2_async_notifier_add_fwnode_subdev will pass remote to camss->notifier. 488 sizeof(*csd)); ... 504 return num_subdevs; -- Regards, Wen [-- Attachment #2: Type: text/plain, Size: 136 bytes --] _______________________________________________ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Cocci] [v2] coccinelle: semantic code search for missing of_node_put 2019-07-04 2:41 ` [Cocci] 答复: Re: [PATCH v2] coccinelle: semantic code search for missingof_node_put wen.yang99 @ 2019-07-04 5:40 ` Markus Elfring 0 siblings, 0 replies; 17+ messages in thread From: Markus Elfring @ 2019-07-04 5:40 UTC (permalink / raw) To: Wen Yang, cocci; +Cc: Yi Wang, Michal Marek, Nicolas Palix, linux-kernel > We will also provide an example written in Python later. Will the code move from the commit description into a file for your next patch version? > We first use this script to find out all the function names to be processed, I am still curious on how the output format selection will become clearer for the potentially desired automatic data conversion. > and then copy these function names into r1. Would this action be performed by another software build script? >>> +@initialize:python@ >>> +@@ >>> + >>> +seen = set() >>> + >>> +def add_if_not_present (p1, p2): >> >> It seems that you would like to use iteration functionality. I am waiting on another constructive answer for this implementation detail. >>> +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? > > As julia pointed out, your current writing is not compiled. * It can be needed for a while to specify the mentioned position variable at an other place. * Would you like to adjust the SmPL coding style here? * Will the application of sort criteria be clarified for such identifier lists? >>> +if (x == NULL || ...) S >>> +... when != e = (T)x >>> + when != true x == NULL … > Our previous version used the "when any" clause, so we need > "when != true x == NULL". I suggest to reconsider further aspects for such constraints. > We can delete this code exclusion specification for this version. I would find another assignment exclusion more appropriate at this place. Regards, Markus _______________________________________________ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2019-07-05 6:46 UTC | newest] Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-06-28 2:58 [Cocci] [PATCH v2] coccinelle: semantic code search for missing of_node_put Wen Yang 2019-06-28 9:38 ` Markus Elfring 2019-06-28 11:07 ` Julia Lawall 2019-06-28 14:16 ` [Cocci] [v2] " Markus Elfring 2019-06-28 14:16 ` Markus Elfring 2019-07-04 3:03 ` [Cocci] [PATCH v2] coccinelle: semantic code search for missingof_node_put wen.yang99 2019-07-04 6:28 ` [Cocci] [v2] coccinelle: semantic code search for missing of_node_put Markus Elfring 2019-07-05 5:29 ` [Cocci] [PATCH v2] coccinelle: semantic code search for missingof_node_put Julia Lawall 2019-07-05 5:57 ` [Cocci] [PATCH v2] coccinelle: semantic code search formissingof_node_put wen.yang99 2019-07-05 6:17 ` Julia Lawall 2019-07-05 6:45 ` [Cocci] [v2] coccinelle: semantic code search for missing of_node_put Markus Elfring 2019-06-29 7:40 ` Markus Elfring 2019-06-29 7:49 ` Julia Lawall 2019-06-29 8:35 ` Markus Elfring 2019-06-29 19:30 ` [Cocci] [v2] Coccinelle: Testing SmPL constraints Markus Elfring 2019-07-04 2:41 ` [Cocci] 答复: Re: [PATCH v2] coccinelle: semantic code search for missingof_node_put wen.yang99 2019-07-04 5:40 ` [Cocci] [v2] coccinelle: semantic code search for missing of_node_put Markus Elfring
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).