Re: [v2] coccinelle: semantic code search for missing of_node_put
>> @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
Re: [v2] coccinelle: semantic code search for missing of_node_put
> 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
Re: [v2] coccinelle: semantic code search for missing of_node_put
> 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
Re: [v2] coccinelle: semantic code search for missing of_node_put
+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
Re: [v2] coccinelle: semantic code search for missing of_node_put
>> +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
Re: [v2] coccinelle: semantic code search for missing of_node_put
>> +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
Re: [v2] coccinelle: semantic code search for missing of_node_put
>> +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
