Re: [Cocci] [PATCH 0/2] Add "use-patchdiff" option
> This patchset adds a feature to enable Coccinelle > to only check all those files in a directory which were > modified. It parses all the files obtained from the > output of "git diff" and checks them against the specified > cocci script. > > An example for passing the "use-patchdiff" option is: How do you think about to use the parameter name “use-files-from-diff”? Regards, Markus ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH v5] coccinelle: api: semantic patch to use pm_runtime_resume_and_get
> v5: print a message with the new function name, as suggested by … Thanks for another small adjustment. Thus I find an other version number consistent in the patch subject. Do you care to influence the run time characteristics a bit more for such a script variant by reducing the number of SmPL rules according to software design possibilities with SmPL disjunctions? Regards, Markus ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH v4] coccinelle: api: semantic patch to use pm_runtime_resume_and_get
… > +msg = "WARNING: opportunity for pm_runtime_get_sync" > +coccilib.org.print_todo(j0[0], msg) … Do you find the following message variant more helpful? +coccilib.org.print_todo(j0[0], +"WARNING: opportunity for replacing pm_runtime_get_sync() by pm_runtime_resume_and_get()") Will the Python code be accordingly adjusted for the SmPL report rule? Regards, Markus ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH v4] coccinelle: api: semantic patch to use pm_runtime_resume_and_get
>… keeps a reference count on failure, …
Would you get into the mood to perform a systematic source code search
for similar function implementations according to resource clean-up?
> v2: better keyword
How do you think about to add the information “wrapper functions” here?
…
> +@r0 depends on patch && !context && !org && !report@
> +expression ret,e;
> +@@
> +
> +- ret = pm_runtime_get_sync(e);
> ++ ret = pm_runtime_resume_and_get(e);
…
I suggest once more to concentrate the specification of such a change
to the desired replacement of the mentioned function name.
+ ret =
+- pm_runtime_get_sync
++ pm_runtime_resume_and_get
+ (e);
…
> + if (ret < 0)
> +- {
> +- pm_runtime_put_noidle(e);
> + S1
> +- }
> + else S2
…
I propose to put this adjustment into a disjunction for the semantic patch
language.
How do you think about to combine five SmPL rules into one?
…
> +* ret@j0 = pm_runtime_get_sync(e);
> + if (ret < 0) {
> + f(...,c,...);
> +* pm_runtime_put_noidle@j1(e);
> + ...
> + } else S
…
Would you like to express in the SmPL context rules that a macro or function
call
is optional here?
Are there any opportunities to consider for the avoidance of duplicate SmPL
code?
…
> +msg = "WARNING: opportunity for pm_runtime_get_sync"
> +coccilib.org.print_todo(j0[0], msg)
…
Can the following Python code be more appropriate?
+coccilib.org.print_todo(j0[0], "WARNING: opportunity for
pm_runtime_resume_and_get")
Would you like to reconsider the message also for the SmPL report rule
accordingly?
Regards,
Markus
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH] coccinelle: api: semantic patch to use pm_runtime_resume_and_get
… > +// Keywords: kwd … Will any information become more helpful and succinct for such a SmPL script variant? Regards, Markus ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Improve the handling of string literals with SmPL
> I'm asking you to put the regexp in a python function.
Will it become easier and more convenient to search for string literals
(and their exclusion according to the discussed transformation approach)?
@initialize:python@
@@
def occurs(x):
return '"' in x
@display@
identifier i =~ "^(?:[A-Z]+_){3,3}[A-Z]+";
expression e : script:python() { occurs(e) };
@@
*#define i e
elfring@Sonne:~/Projekte/PipeWire/lokal> spatch
~/Projekte/Coccinelle/janitor/show_define_usage8.cocci
spa/include/spa/node/type-info.h
…
@@ -35,8 +35,6 @@ extern "C" {
#include
#include
-#define SPA_TYPE_INFO_IO SPA_TYPE_INFO_ENUM_BASE "IO"
-#define SPA_TYPE_INFO_IO_BASE SPA_TYPE_INFO_IO ":"
static const struct spa_type_info spa_type_io[] = {
{ SPA_IO_Invalid, SPA_TYPE_Int, SPA_TYPE_INFO_IO_BASE "Invalid", NULL },
…
How will the software design clarification evolve further?
Regards,
Markus
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Excluding quotes from strings of #define directives
>> I hoped that the specified constraint for the metavariable “e” would mean
>> that expressions which contain a double quotation character should be
>> excluded
>> for my source code analysis approach.
>> Would you like to check the observed software functionality once more?
>
> There is perhaps a problem, but it is surely not necessary to have all of
> this python code around it to see the problem.
I chose this test display so that it can be clearly seen which data were
processes
for the metavariable “e”.
> Please make a minimal example. A rule with a match and a * in front of it
> should be sufficient.
Do you find constraints of the following SmPL script variant easier to clarify?
@display@
identifier i =~ "^(?:[A-Z]+_){3,3}[A-Z]+";
expression e !~ "\"";
@@
*#define i e
Test result:
https://gitlab.freedesktop.org/pipewire/pipewire/-/blob/97b01ed9b01bac7cba68ff11c6bf7ceabcae7f52/spa/include/spa/node/type-info.h#L38
elfring@Sonne:~/Projekte/PipeWire/lokal> spatch
~/Projekte/Coccinelle/janitor/show_define_usage4.cocci
spa/include/spa/node/type-info.h
…
@@ -35,8 +35,6 @@ extern "C" {
#include
#include
-#define SPA_TYPE_INFO_IO SPA_TYPE_INFO_ENUM_BASE "IO"
-#define SPA_TYPE_INFO_IO_BASE SPA_TYPE_INFO_IO ":"
static const struct spa_type_info spa_type_io[] = {
{ SPA_IO_Invalid, SPA_TYPE_Int, SPA_TYPE_INFO_IO_BASE "Invalid", NULL },
…
Regards,
Markus
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Checking support for compound expressions (according to #define directives)
>> I would like to avoid the repetition of parsing efforts as much as possible.
>> Under which circumstances can replacement lists be taken better into account?
>
> Why does my suggestion involve a repetition of parsing effort?
The selection of the applied programming interfaces has got significant
influences
on the run time behaviour.
See also:
https://github.com/coccinelle/coccinelle/issues/200#issuecomment-653775288
> You want to use a regexp.
This view depends on some factors.
I would prefer to search for string literals (and their exclusion) by higher
level means.
> I'm asking you to put the regexp in a python function.
How do you think about to improve the following software situation
besides the application of regular expressions?
@initialize:python@
@@
import re
@display@
identifier i =~ "^(?:[A-Z]+_){3,3}[A-Z]+";
expression e : script:python() { re.match('"', e) };
@@
*#define i e
elfring@Sonne:~/Projekte/PipeWire/lokal> spatch
~/Projekte/Coccinelle/janitor/show_define_usage7.cocci
spa/include/spa/node/type-info.h
…
File "", line 5
coccinelle.result = (re . match ( " , e ))
^
SyntaxError: EOL while scanning string literal
Regards,
Markus
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] changing of_get_mac_address() to pass a buffer
>mac = of_get_mac_address(np); >if (!IS_ERR(mac)) > ether_addr_copy(ndev->dev_addr, mac); > > This would need to be changed to: > >of_get_mac_address(np, ndev->dev_addr); Can such a function call variant fail? How do you think about take any special return values into account? > Maybe (well I'm sure) there is something to improve here. I suggest once more to group another bit of SmPL code like “+ ret_tbd = …” by using nested SmPL disjunctions so that duplicate specifications can be reduced. Regards, Markus ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] Replacing #define directives with the help of SmPL
Hello, Will the software development interests ever evolve in ways so that #define directives can be replaced with the help of the semantic patch language for special source code analysis and transformation approaches? https://github.com/coccinelle/coccinelle/issues/139 Regards, Markus ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] Searching only for header files with the Coccinelle software
Hello, The Coccinelle software can search for header files in addition to source files if the option “--include-headers” was specified. https://github.com/coccinelle/coccinelle/blob/287374196da8c7cfd169e721a2d23f1e462422f1/docs/manual/spatch_options.tex#L43 How can it be achieved that only header files will be searched by this tool for special source code analysis and transformation approaches? Regards, Markus ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Checking evolution according to version 1.1.0
> A new version 1.1.0 has been released. Thanks. I have dared to activate also a current OCaml version by the command “opam switch create 4.12.0”. I have tried to rebuild your software accordingly. elfring@Sonne:~/Projekte/Coccinelle/20160205> make distclean && git checkout master && git pull && ./autogen && ./configure && echo "$(./version.sh)" && grep VERSION=1 Makefile.config Now I stumble on the following error message. elfring@Sonne:~/Projekte/Coccinelle/20160205> LANG=C make world make -C bundles/stdcompat all make[1]: Entering directory '/home/elfring/Projekte/Coccinelle/20160205/bundles/stdcompat' cd stdcompat-current; make && cp *.mli *.cmi *.cmx *.cma *.cmxa *.a *.h .. make[2]: Entering directory '/home/elfring/Projekte/Coccinelle/20160205/bundles/stdcompat/stdcompat-current' make all-am make[3]: Entering directory '/home/elfring/Projekte/Coccinelle/20160205/bundles/stdcompat/stdcompat-current' ocamlfind ocamlopt -c -package seq -package uchar -bin-annot -no-alias-deps -I . -alert -deprecated stdcompat__arg_s.mli -o stdcompat__arg_s.cmi File "stdcompat__arg_s.mli", lines 3-17, characters 0-38: 3 | type spec = Arg.spec = 4 | | Unit of (unit -> unit) 5 | | Bool of (bool -> unit) 6 | | Set of bool ref 7 | | Clear of bool ref ... 14 | | Tuple of spec list 15 | | Symbol of string list * (string -> unit) 16 | | Rest of (string -> unit) 17 | | Expand of (string -> string array). Error: This variant or record definition does not match that of type Arg.spec Constructors number 14 have different names, Rest_all and Expand. make[3]: *** [Makefile:1612: stdcompat__arg_s.cmi] Error 2 … Would you like to point any corresponding adjustments out? Regards, Markus ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH] coccinelle: misc: add swap script
> +- tmp = a; > +- a = b; > +- b = tmp; > ++ swap(a, b); How do you think about to use the following SmPL code variant? * Omission of a few leading space characters * Keeping a semicolon unmodified in a line +-tmp = a; +-a = b; +-b = tmp ++swap(a, b) +; Regards, Markus ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH 1/3] kbuild: do not use scripts/ld-version.sh for checking spatch version
…
> +++ b/scripts/coccicheck
…
> @@ -186,14 +185,11 @@ coccinelle () {
…
> +if [ -n "$REQ" ] && ! { echo "$REQ"; echo "$SPATCH_VERSION"; } | sort
> -CV ; then
Would it make sense to use an other command control operator?
+if [ -n "$REQ" ] && ! { echo "$REQ" && echo "$SPATCH_VERSION"; } | sort
-CV ; then
How do you think about another command variant?
+if [ -n "$REQ" ] && ! printf "%s\n%s\n" "${REQ}" "${SPATCH_VERSION}" |
sort -CV ; then
…
> +++ b/scripts/nsdeps
> @@ -12,11 +12,9 @@ if [ ! -x "$SPATCH" ]; then
…
> +if ! { echo "$SPATCH_REQ_VERSION"; echo "$SPATCH_VERSION"; } | sort -CV ;
> then
Are command alternatives helpful at such a place?
+if ! { echo "$SPATCH_REQ_VERSION" && echo "$SPATCH_VERSION"; } | sort -CV ;
then
Or
+if ! printf "%s\n%s\n" "${SPATCH_REQ_VERSION}" "${SPATCH_VERSION}" | sort -CV
; then
Regards,
Markus
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Checking the influence of an omitted semicolon on a code adjustment
> Why do you want to remove the semicolon? I have shown a transformation example where a function parameter should be replaced by a previous function call. Thus a semicolon should be intentionally be deleted. > If you want to find the call somewhere in the next statement, you can say > > ( > S > & > call( > ... > ) > ) > > where S is a statement metavariable. This SmPL specification variant can be also interesting. I got just interested in the possibility to omit an extra semicolon in my simple change approach. Regards, Markus ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Checking the influence of an omitted semicolon on a code adjustment
>> I have shown a transformation example where a function parameter should be >> replaced >> by a previous function call. >> Thus a semicolon should be intentionally be deleted. > > That makes no sense. This transformation part is working as expected (under constraints) already. > You can't have an expression directly following a statement. > Only statements follow other statements. This information is reasonable. Does the Coccinelle software insist on the specification of another semicolon in the SmPL script for the identification of an adjustable statement? Regards, Markus ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Checking the influence of an omitted semicolon on a code adjustment
>>> You can't have an expression directly following a statement. >>> Only statements follow other statements. >> >> This information is reasonable. >> >> Does the Coccinelle software insist on the specification of another semicolon >> in the SmPL script for the identification of an adjustable statement? > > Yes I imagine that another view can become applicable here. * It is possible to move a function call with a known property into a parameter of a subsequent function/macro call. * I care more for the implementation detail that a following function/macro call exists and less that this source code needs to be an complete statement (together with the usual semicolon at the end). I stumble on further software challenges if I would like to make the simple transformation approach a bit more variable on the number of surrounding parameters. Regards, Markus ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] Checking the influence of an omitted semicolon on a code adjustment
Hello, The following small SmPL script gets successfully parsed by the Coccinelle software. @Replacement@ expression call, input, target; identifier gs; @@ -\( g_string_assign@gs \| g_string_append@gs \) (target, input); call ( - target + gs (target, input) ); But if I would like to omit the semicolon in the last line, I stumble on the error message “incompatible minus and plus code starting on lines 5 and 6”. Can such a software limitation be adjusted? Regards, Markus ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH v2] coccinelle: locks: Add balancedlock.cocci script
> Changes in v2(as suggested by Markus):
Thanks you picked a few suggestions up.
I would appreciate further constructive clarifications.
…
> +++ b/scripts/coccinelle/locks/balancedlock.cocci
…
> +//# False positives may be generated due to locks released within a nested
> +//# function call or a goto block.
> +///
> +// Confidence: Moderate
How good does such information fit together?
> +// Copyright: (C) 2020 Julia Lawall INRIA/LIP6
> +// Copyright: (C) 2020 Sumera Priyadarsini
Does this information indicate that the shown script for the semantic patch
language
was a development result from another collaboration?
…
>+ (
> +mutex_lock@p(E);
> +|
> +read_lock@p(E);
> +|
…
Why did you not reorder the elements of such a SmPL disjunctions according to
an usage incidence (which can be determined by another SmPL script like
“report_lock_calls.cocci”)?
…
> +@balanced2 depends on context || org || report@
> +identifier lock, unlock = {mutex_unlock,
…
Are there any chances to avoid the repetition of the function name list
for this SmPL constraint?
…
> +msg = "This code segment might have an unbalanced lock."
> +coccilib.org.print_todo(j0[0], msg)
Please pass the string literal directly.
+coccilib.org.print_todo(j0[0], "This code segment might have an unbalanced
lock.")
Regards,
Markus
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Checking the handling of spaces before opening parentheses for source code transformations
> If you want to ensure that your generated code has such properties, > you can use the argument --smpl-spacing. It is nice that code additions can be influenced by such a parameter. > If you want something more, you are welcome to implement it. I imagine that adjustments for matched metavariables might need further development considerations. Would there be a need to reformat any expression code (for example)? Which software places would become interesting for extensions of source code layout algorithms? > We only focus on the coding style of the Linux kernel. How are the chances that interests will grow also for source files of GNOME components? Regards, Markus ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Checking the handling of spaces before opening parentheses for source code transformations
> If you want to ensure that your generated code has such properties, you > can use the argument --smpl-spacing. Can there be a need for a related run time parameter for the handling of line breaks? Regards, Markus ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] Checking the handling of spaces before opening parentheses for source code transformations
Hello, The coding style specification for C programs in GNOME contains also the following information. https://developer.gnome.org/programming-guidelines/stable/c-coding-style.html.en#whitespace “… Always put a space before an opening parenthesis but never after: …” How good can the semantic patch language support such a requirement (also for function declarations and corresponding calls) directly? See also a related feature request once more: Make change influence configurable for coding style rules https://github.com/coccinelle/coccinelle/issues/37 Regards, Markus ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Coccinelle: Checking the relevance of parentheses in “git grep”
>> … git grep -l -w \( -e for_each_node_by_type … -e >> for_each_node_with_property \) -- '*.c' >> >> >> How do you think about to omit these parentheses here? > > Does it make a difference? I find that it can be nicer to avoid the passing of unnecessary characters. * The compilation of the affected OCaml source files can eventually benefit from another bit of clean-up, can't it? * The called software components do also not need to fiddle with such extra data then. Regards, Markus ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Coccinelle: Checking the relevance of parentheses in “git grep”
> Will any adjustments become relevant then accordingly? I have found out that the function “interpret” (OCaml code) constructs a command and executes it. https://github.com/coccinelle/coccinelle/blob/3c01dc1696dc5ccfb319673f205e491b572ee0be/parsing_cocci/git_grep.ml#L9 I have tried a corresponding test display out. Thus I have got the impression that desired patterns are passed with extra parentheses. … git grep -l -w \( -e for_each_node_by_type … -e for_each_node_with_property \) -- '*.c' How do you think about to omit these parentheses here? Regards, Markus ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Coccinelle: null: Optimise disjunctions in SmPL script “eno.cocci”
>> This analysis result indicates a clear ranking for such function calls. >> Thus reorder the SmPL disjunction items according to their usage incidence. > > Did you actually test this before and after the change and see a > difference in performance? Would you become interested to configure a representative test environment for safe comparisons of corresponding run time characteristics of the affected software? > On my laptop, I see absolutely no difference, > for the patch mode and for the context mode. Does such information trigger a desire to clarify involved aspects in more detail? Regards, Markus ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Coccinelle: null: Optimise disjunctions in SmPL script “eno.cocci”
>> Would you become interested to configure a representative test environment >> for safe comparisons of corresponding run time characteristics >> of the affected software? > > In what sense could the comparison possibly be unsafe? * Our test systems are obviously different. Thus concerns can be considered for reproducibility of test results on other possible configurations. * We share only a tiny fraction of technical information which would probably be needed for “benchmarks”. > Just use time and run spatch on whatever machine you want. fring@Sonne:~/Projekte/Linux/next-patched> elfring@Sonne:~/Projekte/Linux/next-patched> git checkout next-20201023 && XX=$(date) && time spatch -D patch --timeout 9 --jobs 4 --chunksize 1 --include-headers --no-includes --dir . scripts/coccinelle/null/eno.cocci > ~/Projekte/Bau/Linux/scripts/Coccinelle/call_checks/20201023/eno1.diff 2> ~/Projekte/Bau/Linux/scripts/Coccinelle/call_checks/20201023/eno1-errors.txt; YY=$(date) && echo "$XX | $YY" … real2m54,266s user10m15,553s sys 0m4,004s So 25. Okt 20:53:56 CET 2020 | So 25. Okt 20:56:51 CET 2020 elfring@Sonne:~/Projekte/Linux/next-patched> git checkout next-20201023 && XX=$(date) && time spatch -D context --timeout 9 --jobs 4 --chunksize 1 --include-headers --no-includes --dir . scripts/coccinelle/null/eno.cocci > ~/Projekte/Bau/Linux/scripts/Coccinelle/call_checks/20201023/eno2.txt 2> ~/Projekte/Bau/Linux/scripts/Coccinelle/call_checks/20201023/eno2-errors.txt; YY=$(date) && echo "$XX | $YY" … real2m38,494s user9m39,364s sys 0m4,094s So 25. Okt 20:58:05 CET 2020 | So 25. Okt 21:00:44 CET 2020 elfring@Sonne:~/Projekte/Linux/next-patched> git checkout optimise_disjunction_in_eno.cocci-1 && XX=$(date) && time spatch -D patch --timeout 9 --jobs 4 --chunksize 1 --include-headers --no-includes --dir . scripts/coccinelle/null/eno.cocci > ~/Projekte/Bau/Linux/scripts/Coccinelle/call_checks/20201023/eno3.diff 2> ~/Projekte/Bau/Linux/scripts/Coccinelle/call_checks/20201023/eno3-errors.txt; YY=$(date) && echo "$XX | $YY" … real2m46,097s user10m15,467s sys 0m3,984s So 25. Okt 21:00:56 CET 2020 | So 25. Okt 21:03:42 CET 2020 elfring@Sonne:~/Projekte/Linux/next-patched> git checkout optimise_disjunction_in_eno.cocci-1 && XX=$(date) && time spatch -D context --timeout 9 --jobs 4 --chunksize 1 --include-headers --no-includes --dir . scripts/coccinelle/null/eno.cocci > ~/Projekte/Bau/Linux/scripts/Coccinelle/call_checks/20201023/eno4.txt 2> ~/Projekte/Bau/Linux/scripts/Coccinelle/call_checks/20201023/eno4-errors.txt; YY=$(date) && echo "$XX | $YY" … real2m41,472s user9m37,492s sys 0m3,834s So 25. Okt 21:03:56 CET 2020 | So 25. Okt 21:06:37 CET 2020 > Intel(R) Core(TM) i5-6200U CPU @ 2.30GHz AMD Phenom(tm) II X4 850 Processor Will any other aspects become relevant? Regards, Markus ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [PATCH] Coccinelle: api: Optimise disjunction in SmPL script “pm_runtime.cocci”
From: Markus Elfring Date: Sun, 25 Oct 2020 14:10:58 +0100 A disjunction is applied by this script for the semantic patch language. This construct uses short-circuit evaluation. It has got the consequence that the last element of the specified condition will only be checked if all previous parts did not match. Such a technical detail leads to a recommended ordering of condition parts if you would like to care for optimal run time characteristics of SmPL code. An usage incidence was determined for the specified identifiers in source files from the software “Linux next-20201023” by another SmPL script. This analysis result indicated that a few functions were called more frequent than others. Thus reorder the SmPL disjunction items partly according to their usage incidence. Signed-off-by: Markus Elfring --- scripts/coccinelle/api/pm_runtime.cocci | 40 + 1 file changed, 21 insertions(+), 19 deletions(-) diff --git a/scripts/coccinelle/api/pm_runtime.cocci b/scripts/coccinelle/api/pm_runtime.cocci index 1ccce3fd00b8..7d9908cf0803 100644 --- a/scripts/coccinelle/api/pm_runtime.cocci +++ b/scripts/coccinelle/api/pm_runtime.cocci @@ -21,25 +21,27 @@ expression ret; position p; @@ ( -ret@p = \(pm_runtime_idle\| - pm_runtime_suspend\| - pm_runtime_autosuspend\| - pm_runtime_resume\| - pm_request_idle\| - pm_request_resume\| - pm_request_autosuspend\| - pm_runtime_get\| - pm_runtime_get_sync\| - pm_runtime_put\| - pm_runtime_put_autosuspend\| - pm_runtime_put_sync\| - pm_runtime_put_sync_suspend\| - pm_runtime_put_sync_autosuspend\| - pm_runtime_set_active\| - pm_schedule_suspend\| - pm_runtime_barrier\| - pm_generic_runtime_suspend\| - pm_generic_runtime_resume\)(...); + ret@p = +(pm_runtime_get_sync +|pm_runtime_set_active +|pm_runtime_put_sync_autosuspend +|pm_runtime_put_sync +|pm_runtime_get +|pm_runtime_put +|pm_generic_runtime_suspend +|pm_generic_runtime_resume +|pm_request_autosuspend +|pm_request_idle +|pm_request_resume +|pm_runtime_autosuspend +|pm_runtime_barrier +|pm_runtime_idle +|pm_runtime_put_autosuspend +|pm_runtime_put_sync_suspend +|pm_runtime_resume +|pm_runtime_suspend +|pm_schedule_suspend +)(...); ... IS_ERR_VALUE(ret) ... -- 2.29.1 ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Coccinelle: null: Optimise disjunctions in SmPL script “eno.cocci”
> In the patch case, the user and system time are essentially identical. > In the context case, the difference in user time is 2 seconds out of 9.5 > minutes, 0.3%. This was just a single test run example. > In the patch case, the real time is a bit slower. I wonder about such an interpretation. > But I believe that this is due to the time to read in the data from the file > system. I assume that I could reduce such an influence a bit if I would put (my) Linux repository into a RAM disk. > I also had a number like that, but the difference disappeared > when I reran it afterwards, which meant running that case in the same > conditions > as the others. > > In the context case, the real time is slightly slower with your patch. I guess that the small variations from such a test approach can eventually become more interesting. > So I see no compelling evidence for making the change. This can be. Should any other differences become more significant? Regards, Markus ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [PATCH] Coccinelle: null: Optimise disjunctions in SmPL script “eno.cocci”
From: Markus Elfring Date: Sun, 25 Oct 2020 18:54:36 +0100 A disjunction is applied by this script for the semantic patch language. This construct uses short-circuit evaluation. It has got the consequence that the last element of the specified condition will only be checked if all previous parts did not match. Such a technical detail leads to a recommended ordering of condition parts if you would like to care for optimal run time characteristics of SmPL code. An usage incidence was determined for the specified identifiers in source files from the software “Linux next-20201023” by another SmPL script. See also: Determination of an usage statistic for memory allocation calls https://lore.kernel.org/cocci/2774601.u91sIFNy1E@sonne/ This analysis result indicates a clear ranking for such function calls. Thus reorder the SmPL disjunction items according to their usage incidence. Signed-off-by: Markus Elfring --- scripts/coccinelle/null/eno.cocci | 15 +++ 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/scripts/coccinelle/null/eno.cocci b/scripts/coccinelle/null/eno.cocci index 81584ff87956..969cab5116a9 100644 --- a/scripts/coccinelle/null/eno.cocci +++ b/scripts/coccinelle/null/eno.cocci @@ -17,8 +17,16 @@ virtual report @depends on patch@ expression x,E; @@ - -x = \(kmalloc\|kzalloc\|kcalloc\|kmem_cache_alloc\|kmem_cache_zalloc\|kmem_cache_alloc_node\|kmalloc_node\|kzalloc_node\)(...) + x = +(kzalloc +|kmalloc +|kcalloc +|kmem_cache_alloc +|kmem_cache_zalloc +|kzalloc_node +|kmalloc_node +|kmem_cache_alloc_node +)(...) ... when != x = E - IS_ERR(x) + !x @@ -27,8 +35,7 @@ x = \(kmalloc\|kzalloc\|kcalloc\|kmem_cache_alloc\|kmem_cache_zalloc\|kmem_cache expression x,E; position p1,p2; @@ - -*x = \(kmalloc@p1\|kzalloc@p1\|kcalloc@p1\|kmem_cache_alloc@p1\|kmem_cache_zalloc@p1\|kmem_cache_alloc_node@p1\|kmalloc_node@p1\|kzalloc_node@p1\)(...) +*x = \(kzalloc@p1\|kmalloc@p1\|kcalloc@p1\|kmem_cache_alloc@p1\|kmem_cache_zalloc@p1\|kzalloc_node@p1\|kmalloc_node@p1\|kmem_cache_alloc_node@p1\)(...) ... when != x = E * IS_ERR@p2(x) -- 2.29.1 ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Adjusting replacement lists with SmPL?
I'd like to add a statement after another within a preprocessor expression, How do you think about to refer to a “#define directive”? but spatch adds the line without an escape (backslash). I imagine that we stumble on another target conflict here. https://github.com/coccinelle/coccinelle/issues/139 Do you really want to adjust a bit of text according to a preprocessing definition? #define X(a) x(a); (I know the above is not technically correct but it's super common.) I stumble on understanding difficulties for this information. Would you like to clarify the knowledge about correctness a bit more? @@ expression e; @@ x(e); + y(e); How should the scope be specified that a change should be performed only for preprocessor code (replacement lists for your transformation approach)? I can think of two solutions, if an expression is inside a preprocessor statement: add a backslash before every newline, or skip the newline. Would you like to choose the preferred coding style for such an use case? Regards, Markus ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Determination of an usage statistic for memory allocation calls
> > Can such facts influence the specification of efficient SmPL disjunctions > > another bit? > > On my machine, putting the three functions that you have foudn to be the > most frequent at the end of each disjunction has no impact on the performance. I propose to reconsider this view. > So what do you suggest? 1. I would appreciate if you would share more technical details about your test environment for a safer comparison. 2. The observed software behaviour can be clarified further, can't it? Aspects like the following can trigger corresponding development considerations. * “noticable” differences (depending on the run time environment) * measurable effects * mathematical properties of an algorithm Regards, Markus ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Determination of an usage statistic for memory allocation calls
> …
> > +E = \(kmalloc\|kzalloc\|krealloc\|kcalloc\|
> > + kmalloc_node\|kzalloc_node\|kmalloc_array\|
> > + kmalloc_array_node\|kcalloc_node\)(...)@kok
> …
>
> How do you think about the possibility for any adjustments according to the
> order
> of the mentioned function names in proposed disjunctions for the semantic
> patch language?
I would like to share another source code analysis approach.
I hope that this contribution can trigger further helpful software development
ideas.
@initialize:python@
@@
import sys
def write_identifier(source, call):
names = []
for x in source:
names.append(call)
sys.stdout.write("\n".join(names) + "\n")
@find1@
expression e;
identifier call, x;
position pos;
type rt;
@@
rt x(...)
{
<+...
e =@pos
(kzalloc@call
|kmalloc@call
|kcalloc@call
|kmalloc_array@call
|kmemdup@call
|kstrdup@call
|vmalloc@call
|vzalloc@call
|kzalloc_node@call
|kvmalloc@call
|krealloc@call
|kmalloc_node@call
|kcalloc_node@call
|__vmalloc@call
|vmalloc_user@call
|vzalloc_node@call
|vmalloc_32@call
|__vmalloc_node_range@call
|vmalloc_node@call
|kmalloc_array_node@call
|__vmalloc_node@call
|vmalloc_32_user@call
|vmalloc_exec@call
)(...)
...+>
}
@script:python collection1@
call << find1.call;
place << find1.pos;
@@
write_identifier(place, call)
@find2@
identifier call, var, x;
position pos;
type rt, vt;
@@
rt x(...)
{
<+...
vt var =@pos
(kzalloc@call
|kmalloc@call
|kcalloc@call
|kmalloc_array@call
|kmemdup@call
|kstrdup@call
|vmalloc@call
|vzalloc@call
|kzalloc_node@call
|kvmalloc@call
|krealloc@call
|kmalloc_node@call
|kcalloc_node@call
|__vmalloc@call
|vmalloc_user@call
|vzalloc_node@call
|vmalloc_32@call
|__vmalloc_node_range@call
|vmalloc_node@call
|kmalloc_array_node@call
|__vmalloc_node@call
|vmalloc_32_user@call
|vmalloc_exec@call
)(...);
...+>
}
@script:python collection2@
call << find2.call;
place << find2.pos;
@@
write_identifier(place, call)
Test result:
elfring@Sonne:~/Projekte/Linux/next-patched> git checkout next-20201016 &&
XX=$(date) && time spatch --timeout 23 --python python3 --jobs 4 --chunksize 1
--include-headers --no-includes --dir .
~/Projekte/Coccinelle/janitor/report_memory_allocation_calls4.cocci 2>
~/Projekte/Bau/Linux/scripts/Coccinelle/call_checks/20201016/report_memory_allocation_calls4-errors.txt
| echo "$(echo 'call' && cat)" | csvsql --query 'select call, count(*) from
stdin group by call order by count(*) desc'; YY=$(date) && echo "$XX | $YY"
…
call,count(*)
kzalloc,12652
kmalloc,4902
kcalloc,2564
kmalloc_array,859
kmemdup,797
kstrdup,469
vmalloc,405
vzalloc,359
kzalloc_node,177
kvmalloc,154
krealloc,151
kmalloc_node,49
kcalloc_node,44
__vmalloc,34
vmalloc_user,28
vzalloc_node,21
vmalloc_32,9
__vmalloc_node_range,8
vmalloc_node,7
kmalloc_array_node,5
__vmalloc_node,4
vmalloc_32_user,1
real22m25,049s
user84m11,257s
sys 0m12,168s
So 18. Okt 16:55:08 CEST 2020 | So 18. Okt 17:17:33 CEST 2020
The log file contains the information “9211 files match”.
Can such facts influence the specification of efficient SmPL disjunctions
another bit?
Regards,
Markus
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Determination of an usage statistic for macro calls “for_each…node…”
> Would you like to look for software configuration alternatives for better
> parallel data processing?
I would like to share another source code analysis approach.
I hope that this contribution can trigger further helpful software development
ideas.
@initialize:python@
@@
import sys
def write_identifier(source, loop):
names = []
for x in source:
names.append(loop)
sys.stdout.write("\n".join(names) + "\n")
@find@
identifier fe, x;
iterator name for_each_node_by_name,
for_each_node_by_type,
for_each_node_with_property,
for_each_matching_node,
for_each_matching_node_and_match,
for_each_compatible_node,
for_each_child_of_node,
for_each_available_child_of_node;
position pos;
statement s;
type t;
@@
t x(...)
{
<+...
(for_each_child_of_node@fe@pos(...) s
|for_each_available_child_of_node@fe@pos(...) s
|for_each_compatible_node@fe@pos(...) s
|for_each_node_by_name@fe@pos(...) s
|for_each_node_by_type@fe@pos(...) s
|for_each_matching_node@fe@pos(...) s
|for_each_matching_node_and_match@fe@pos(...) s
|for_each_node_with_property@fe@pos(...) s
)
...+>
}
@script:python collection@
fe << find.fe;
place << find.pos;
@@
write_identifier(place, fe)
Test result:
elfring@Sonne:~/Projekte/Linux/next-patched> git checkout next-20201016 &&
XX=$(date) && time spatch --python python3 --jobs 4 --include-headers
--no-includes --dir .
~/Projekte/Coccinelle/janitor/report_for_each_node_macro_calls5.cocci | echo
"$(echo 'call' && cat)" | csvsql --query 'select call, count(*) from stdin
group by call'; YY=$(date) && echo "$XX | $YY"
…
523 files match
…
call,count(*)
for_each_available_child_of_node,158
for_each_child_of_node,359
for_each_compatible_node,80
for_each_matching_node,22
for_each_matching_node_and_match,16
for_each_node_by_name,59
for_each_node_by_type,53
for_each_node_with_property,6
real0m47,779s
user2m19,285s
sys 0m1,541s
So 18. Okt 13:13:02 CEST 2020 | So 18. Okt 13:13:50 CEST 2020
Can such facts influence the specification of efficient SmPL disjunctions any
more?
Regards,
Markus
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Determination of an usage statistic for macro calls “for_each…node…”
> …
> > +(
> > +for_each_node_by_name(n,e1) S
> > +|
> …
> > +|
> > +for_each_node_with_property(n,e1) S
> > +)
> …
>
>
> Do you indicate any occurrence frequencies or probabilities for the mentioned
> macro calls
> by the ordering in this disjunction for the semantic patch language?
I would like to share another source code analysis approach.
I hope that this contribution can trigger further helpful software development
ideas.
@initialize:python@
@@
import sys, sqlalchemy
sys.stderr.write("\n".join( ("Using SQLAlchemy version:",
sqlalchemy.__version__) ))
sys.stderr.write("\n")
from sqlalchemy import Column, Integer, String, create_engine
from sqlalchemy.ext.declarative import declarative_base
from sqlalchemy.orm import sessionmaker
engine = create_engine("sqlite:///:memory:", echo=False)
base = declarative_base()
class action(base):
__tablename__ = "macros"
name = Column(String, primary_key=True)
source_file = Column(String, primary_key=True)
macro = Column(String, primary_key=True)
line = Column(Integer, primary_key=True)
column = Column(Integer, primary_key=True)
def __repr__(self):
return "" % (self.name,
self.source_file,
self.macro,
self.line,
self.column)
configured_session = sessionmaker(bind=engine)
session = configured_session()
base.metadata.create_all(engine)
def store_position(source, loop):
"""Add data to an internal table."""
for place in source:
entry = action(name = place.current_element,
source_file = place.file,
macro = loop,
line = place.line,
column = int(place.column) + 1)
session.add(entry)
@find@
identifier for_loop, work;
iterator name for_each_node_by_name,
for_each_node_by_type,
for_each_node_with_property,
for_each_matching_node,
for_each_matching_node_and_match,
for_each_compatible_node,
for_each_child_of_node,
for_each_available_child_of_node;
position pos;
statement s;
type t;
@@
t work(...)
{
<+...
(for_each_node_by_name@for_loop@pos(...) s
|for_each_node_by_type@for_loop@pos(...) s
|for_each_matching_node@for_loop@pos(...) s
|for_each_node_with_property@for_loop@pos(...) s
|for_each_compatible_node@for_loop@pos(...) s
|for_each_matching_node_and_match@for_loop@pos(...) s
|for_each_child_of_node@for_loop@pos(...) s
|for_each_available_child_of_node@for_loop@pos(...) s
)
...+>
}
@script:python collection@
fl << find.for_loop;
place << find.pos;
@@
store_position(place, fl)
@finalize:python@
@@
session.commit()
from sqlalchemy import func
entries = session.query(func.count()).select_from(action).scalar()
if entries > 0:
from sqlalchemy.sql import literal_column
delimiter = "|"
sys.stdout.write(delimiter.join(['"source file"', 'macro', 'incidence']))
sys.stdout.write("\r\n")
mark = ['"', '', '"']
for file, \
macro, \
incidence in session.query(action.source_file,
action.macro,
func.count(literal_column("*"))
) \
.group_by(action.source_file, action.macro) \
.order_by(action.source_file,
func.count(literal_column("*")).desc()):
mark[1] = file
sys.stdout.write(delimiter.join([''.join(mark), macro, str(incidence)]))
sys.stdout.write("\r\n")
sys.stdout.write("=\r\n")
sys.stdout.write(delimiter.join(['macro', 'incidence']))
sys.stdout.write("\r\n")
for macro, \
incidence in session.query(action.macro,
func.count(literal_column("*"))) \
.group_by(action.macro) \
.order_by(func.count(literal_column("*")).desc()):
sys.stdout.write(macro + delimiter + str(incidence))
sys.stdout.write("\r\n")
else:
sys.stderr.write("No result for this analysis!\n")
Test result:
elfring@Sonne:~/Projekte/Linux/next-patched> git checkout next-20201016 &&
XX=$(date) && time spatch --python $(which python3) --dir .
~/Projekte/Coccinelle/janitor/report_for_each_node_macro_calls.cocci;
YY=$(date) && echo "$XX | $YY"
…
Using SQLAlchemy version:
1.3.19
518 files match
…
=
macro|incidence
for_each_child_of_node|357
for_each_available_child_of_node|157
for_each_compatible_node|79
for_each_node_by_name|55
for_each_node_by_type|53
for_each_matching_node|22
for_each_matching_node_and_match|16
for_each_node_with_property|6
real3m26,039s
user2m3,453s
sys 0m5,041s
Sa 17. Okt 07:00:42 CEST 2020 | Sa 17. Okt 07:04:08 CEST 2020
Can such facts influence the specification of efficient SmPL disjunctions any
more?
Would you like to look for software configuration alternatives for better
parallel data processing?
Regards,
Markus
Re: [Cocci] [PATCH v8] coccinelle: api: add kfree_mismatch script
> > How do you think about the possibility for any adjustments according to the > > order > > of the mentioned function names in proposed disjunctions for the semantic > > patch language? > > Please think about this for 5 seconds. Maybe there are 2000 calls to > these allocation functions, and maybe there are a million function calls > in the files that contain these calls. Would you become interested to check the usage statistics in more detail? > Microscopically optimizing the treatment of 2000 calls is not going to do > anything > to help the overall runtime, which depends on matching all > of the above function names against the one million overall calls. I got an other software understanding for the evaluation characteristics of discussed SmPL scripts. > > Can any additional identifiers become relevant? > > If you have other names to suggest, please do. If you don't have other > names to suggest, then please stop asking such rhetorical questions. I suggest to look at further possibilities so that more function call combinations can be checked automatically. How do you think about approaches to determine relevant properties in systematic ways (besides listing involved identifiers explicitly)? Regards, Markus ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH v8] coccinelle: api: add kfree_mismatch script
… > +E = \(kmalloc\|kzalloc\|krealloc\|kcalloc\| > + kmalloc_node\|kzalloc_node\|kmalloc_array\| > + kmalloc_array_node\|kcalloc_node\)(...)@kok … How do you think about the possibility for any adjustments according to the order of the mentioned function names in proposed disjunctions for the semantic patch language? Can any additional identifiers become relevant? Regards, Markus ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] Explanation for kinds of SmPL rules
Hello, The manual for the semantic patch language mentions the grammar item “rulekind”. https://github.com/coccinelle/coccinelle/blob/730dbb034559b3e549ec0b2973cd0400a3fa072f/docs/manual/cocci_syntax.tex#L116 I would appreciate further information for the safe application of such a technical detail. Regards, Markus ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH] coccinelle: iterators: Add for_each_child.cocci script
> +@ruletwo depends on patch && !context && !org && !report@
How do you think about to combine code from two SmPL rules
by using another SmPL disjunction like the following?
@addition_rule depends on patch && !context && !org && !report@
local idexpression r.n;
expression e,e1;
expression list [r.n1] es;
iterator r.i,i1,i2;
statement S,S2;
@@
(
i(es,n,...) {
...
(of_node_put(n);
|e = n
|return n;
|i1(...,n,...) S
|
+of_node_put(n);
?return ...;
)
... when any
}
|
i(es,n,...) {
...
(of_node_put(n);
|e = n
|i1(...,n,...) S
|
+of_node_put(n);
?break;
)
... when any
}
... when != n
when strict
(n = e1;
|
?i2(...,n,...) S2
)
)
Regards,
Markus
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [PATCH] Coccinelle: api: Add SmPL script “use_platform_get_irq.cocci”
From: Markus Elfring
Date: Wed, 23 Sep 2020 18:30:25 +0200
A wrapper function is available since the commit
7723f4c5ecdb8d832f049f8483beb0d1081cedf6
("driver core: platform: Add an error message to platform_get_irq*()").
Provide design options for the adjustment of affected source code
by the means of the semantic patch language (Coccinelle software).
Signed-off-by: Markus Elfring
---
.../coccinelle/api/use_platform_get_irq.cocci | 107 ++
1 file changed, 107 insertions(+)
create mode 100644 scripts/coccinelle/api/use_platform_get_irq.cocci
diff --git a/scripts/coccinelle/api/use_platform_get_irq.cocci
b/scripts/coccinelle/api/use_platform_get_irq.cocci
new file mode 100644
index ..0a0b27a3cd1a
--- /dev/null
+++ b/scripts/coccinelle/api/use_platform_get_irq.cocci
@@ -0,0 +1,107 @@
+// SPDX-License-Identifier: GPL-2.0
+/// Simplify a function call combination by using a known wrapper function.
+//
+// Keywords: wrapper function conversion IRQ resources
+// Confidence: High
+// Options: --no-includes --include-headers
+
+virtual context, patch, report, org
+
+@depends on context@
+expression device, error_code, index, irq, resource, x;
+identifier ri;
+type t;
+@@
+(
+*resource = platform_get_resource(device, IORESOURCE_IRQ, index);
+*if (resource == NULL)
+*{
+* dev_err(>dev, ...);
+* return error_code;
+*}
+ irq =
+* resource->start
+ ;
+|
+*t ri = platform_get_resource(device, IORESOURCE_IRQ, index);
+ ... when != ri
+ when != device = x
+*if (ri == NULL)
+*{
+* dev_err(>dev, ...);
+* return error_code;
+*}
+ irq =
+* ri->start
+ ;
+)
+
+@depends on patch@
+expression device, error_code, index, irq, resource, x;
+identifier ri;
+type t;
+@@
+(
+-resource = platform_get_resource(device, IORESOURCE_IRQ, index);
+-if (resource == NULL)
+-{
+- dev_err(>dev, ...);
+- return error_code;
+-}
+ irq =
+- resource->start
++ platform_get_irq(device, index)
+ ;
++if (irq < 0)
++ return irq;
+|
+-t ri = platform_get_resource(device, IORESOURCE_IRQ, index);
+ ... when != ri
+ when != device = x
+-if (ri == NULL)
+-{
+- dev_err(>dev, ...);
+- return error_code;
+-}
+ irq =
+- ri->start
++ platform_get_irq(device, index)
+ ;
++if (irq < 0)
++ return irq;
+)
+
+@or depends on org || report@
+expression device, error_code, index, irq, resource, x;
+identifier ri;
+position p;
+type t;
+@@
+(resource = platform_get_resource(device, IORESOURCE_IRQ, index);
+ if (resource == NULL)
+ {
+dev_err(>dev, ...);
+return error_code;
+ }
+ irq = resource->start@p;
+|
+ t ri = platform_get_resource(device, IORESOURCE_IRQ, index);
+ ... when != ri
+ when != device = x
+ if (ri == NULL)
+ {
+dev_err(>dev, ...);
+return error_code;
+ }
+ irq = ri->start@p;
+)
+
+@script:python depends on org@
+p << or.p;
+@@
+coccilib.org.print_todo(p[0], "WARNING: opportunity for platform_get_irq()")
+
+@script:python depends on report@
+p << or.p;
+@@
+coccilib.report.print_report(p[0], "WARNING: opportunity for
platform_get_irq()")
--
2.28.0
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH] coccinelle: iterators: Add for_each_child.cocci script
> +// Confidence: High Will an addition like the following be helpful? +// Options: --no-includes --include-headers > +@r@ > +local idexpression n; > +expression e1,e2; > +iterator name for_each_node_by_name, for_each_node_by_type, > +for_each_compatible_node, for_each_matching_node, > +for_each_matching_node_and_match, for_each_child_of_node, > +for_each_available_child_of_node, for_each_node_with_property; > +iterator i; > +statement S; > +expression list [n1] es; > +@@ > + > +( > +( > +for_each_node_by_name(n,e1) S > +| > +for_each_node_by_type(n,e1) S > +| > +for_each_compatible_node(n,e1,e2) S > +| > +for_each_matching_node(n,e1) S > +| > +for_each_matching_node_and_match(n,e1,e2) S > +| > +for_each_child_of_node(e1,n) S > +| > +for_each_available_child_of_node(e1,n) S > +| > +for_each_node_with_property(n,e1) S > +) > +& > +i(es,n,...) S > +) How do you think about to use the following SmPL code variant? @r@ local idexpression n; expression e1, e2; expression list [n1] es; iterator name for_each_node_by_name, for_each_node_by_type, for_each_node_with_property, for_each_matching_node, for_each_matching_node_and_match, for_each_compatible_node, for_each_child_of_node, for_each_available_child_of_node; iterator i; statement S; @@ ( (for_each_node_by_name(n,e1) S |for_each_node_by_type(n,e1) S |for_each_matching_node(n,e1) S |for_each_node_with_property(n,e1) S |for_each_compatible_node(n,e1,e2) S |for_each_matching_node_and_match(n,e1,e2) S |for_each_child_of_node(e1,n) S |for_each_available_child_of_node(e1,n) S ) (es,n,...) S ) Regards, Markus ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Checking adjustments for pointer resets with SmPL
> I have noticed the update suggestion “USB: quirks: simplify quirk handling”.
> https://lore.kernel.org/linux-usb/20200921113039.ga19...@duo.ucw.cz/
> https://lore.kernel.org/patchwork/patch/1308991/
I would like to point out that an acceptable patch can be generated for
the implementation of the function “quirks_param_set” according to
a known transformation pattern by the following SmPL script variant.
@adjustment@
identifier allocate, object, release;
type t;
@@
-if (object)
-{
-release(object);
-object = NULL;
-}
+release(object);
object = allocate(..., sizeof(t), ...);
Regards,
Markus
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] Checking adjustments for pointer resets with SmPL
Hello,
I have noticed the update suggestion “USB: quirks: simplify quirk handling”.
https://lore.kernel.org/linux-usb/20200921113039.ga19...@duo.ucw.cz/
https://lore.kernel.org/patchwork/patch/1308991/
Thus I converted a patch hunk into the following script variant for
the semantic patch language (according to the software “Coccinelle
1.0.8-00177-g28737419”).
@adjustment@
identifier allocate, object, release;
@@
-if (object)
-{
-release(object);
-object = NULL;
-}
+release(object);
object = allocate(..., sizeof(...), ...);
Now I wonder at the moment why a known patch is not generated.
(An error message is not displayed.)
elfring@Sonne:~/Projekte/Linux/next-patched> spatch drivers/usb/core/quirks.c
~/Projekte/Coccinelle/janitor/simplify_pointer_reset2.cocci
Regards,
Markus
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] Searching for functions with non-internal linkage
Hello,
I have tried another tiny script variant out for the semantic patch language
(according to the software combination “Coccinelle 1.0.8-00177-g28737419”).
@find@
identifier i;
type t;
@@
t i(...)
{ ... }
@find_static@
identifier find.i;
type find.t;
@@
static t i(...)
{ ... }
@display depends on !find_static@
identifier find.i;
type find.t;
@@
*t i(...)
{ ... }
This source code analysis approach is generally working in the way
it is designed.
Can the same data processing results be achieved also with a single SmPL rule?
Regards,
Markus
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [RFC PATCH] scripts: coccicheck: Improve error feedback when coccicheck fails
…
> +++ b/scripts/coccicheck
> @@ -126,8 +126,14 @@ run_cmd_parmap() {
> if [ $VERBOSE -ne 0 ] ; then
> echo "Running ($NPROC in parallel): $@"
> fi
> - echo $@ >>$DEBUG_FILE
> - $@ 2>>$DEBUG_FILE
> + if [ "$DEBUG_FILE" != "/dev/null" -a "$DEBUG_FILE" != "" ]; then
…
How do you think about to use the following check variant?
+ if [ "${DEBUG_FILE}" != '/dev/null' -a "${DEBUG_FILE}" != '' ]; then
Regards,
Markus
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [RFC PATCH 1/3] parsing_c: includes_cache: Implement a name cache
> Implement a name cache and includes dependency graph to optimize > performance for recursive parsing of header files. Can such information trigger any more evolution besides the contributed OCaml source code? > The names of the above are stored in a "name cache", i.e. a hashtable > to map the name to the files it is declared in. How much does hashing matter here? > - A dependency graph is built to determine dependencies between all the > files in the codebase. Can such information indicate a need for its own programming interface? > - In the type annotation phase of the C subsystem, if a function call, > struct/union field or identifier is encountered, the type of which is > not known to the annoter, the name cache is checked for the name. Is there anything in common with symbol tables? Regards, Markus ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [RFC PATCH 0/3] parsing_c: Optimize recursive header file parsing
> This patch series aims to optimize performance for recursively parsing > header files in Coccinelle. I am curious how you got encouraged to pick such a software development challenge up. > it would take close to 7 hours to complete. This is unfortunate. How do you think about to offer any more information (besides the mentioned processor) for further benchmarking purposes? https://github.com/coccinelle/coccinelle/issues/133 > The optimization that this patch series implements reduces that time to 1 > hour. Such a scale of improvement is impressive. > - Skipping unparsable locations in header files: Skipping top-level items > in a header file that cannot be parsed. … Will any further software evolution happen according to a topic like “Exclusion of unsupported source code parts”? https://github.com/coccinelle/coccinelle/issues/20 > - Recursively parse all header files only once and build a large type > environment. Use the dependency graph to determine reachability. … Are you looking for the support of header file “precompilation”? Regards, Markus ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH v2] scripts: coccicheck: Do not use shift command when rule is specified
> Modify coccicheck to use the shift command only when > number of shell arguments is not zero. I suggest to add the tag “Fixes” to the commit message. > Changes in V2: > - Fix spelling errors as suggested by Markus Elfring Would you like to adjust the last word in the previous patch subject accordingly? Regards, Markus ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [PATCH v2] Coccinelle: api: Add SmPL script “use_devm_platform_get_and_ioremap_resource.cocci”
From: Markus Elfring
Date: Mon, 7 Sep 2020 18:38:04 +0200
Another wrapper function is available since the commit
890cc39a879906b63912482dfc41944579df2dc6
("drivers: provide devm_platform_get_and_ioremap_resource()").
Provide safe design options for the adjustment of affected source code
by the means of the semantic patch language (Coccinelle software).
Signed-off-by: Markus Elfring
---
v2:
Julia Lawall requested to omit case distinctions (disjunctions) from
the first SmPL script.
The usage of different expression metavariables for the first parameter
of function calls was too questionable for the proposed source
code transformation.
...vm_platform_get_and_ioremap_resource.cocci | 48 +++
1 file changed, 48 insertions(+)
create mode 100644
scripts/coccinelle/api/use_devm_platform_get_and_ioremap_resource.cocci
diff --git
a/scripts/coccinelle/api/use_devm_platform_get_and_ioremap_resource.cocci
b/scripts/coccinelle/api/use_devm_platform_get_and_ioremap_resource.cocci
new file mode 100644
index ..319583716ac8
--- /dev/null
+++ b/scripts/coccinelle/api/use_devm_platform_get_and_ioremap_resource.cocci
@@ -0,0 +1,48 @@
+// SPDX-License-Identifier: GPL-2.0
+/// Simplify a function call combination by using a known wrapper function.
+//
+// Keywords: wrapper function conversion ioremap resources
+// Confidence: High
+
+virtual context, patch, report, org
+
+@display depends on context@
+expression base, device, index, resource;
+@@
+*resource = platform_get_resource(device, IORESOURCE_MEM, index);
+ base =
+* devm_ioremap_resource
+ (>dev, resource);
+
+@replacement depends on patch@
+expression base, device, index, resource;
+@@
+-resource = platform_get_resource(device, IORESOURCE_MEM, index);
+ base =
+- devm_ioremap_resource
++ devm_platform_get_and_ioremap_resource
+ (
+- &
+ device
+-->dev
+ ,
+- resource
++ index,
+ );
+
+@or depends on org || report@
+expression base, device, index, resource;
+position p;
+@@
+ resource = platform_get_resource(device, IORESOURCE_MEM, index);
+ base = devm_ioremap_resource@p(>dev, resource);
+
+@script:python to_do depends on org@
+p << or.p;
+@@
+coccilib.org.print_todo(p[0], "WARNING: opportunity for
devm_platform_get_and_ioremap_resource()")
+
+@script:python reporting depends on report@
+p << or.p;
+@@
+coccilib.report.print_report(p[0], "WARNING: opportunity for
devm_platform_get_and_ioremap_resource()")
--
2.28.0
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH] Coccinelle: api: Add SmPL script “use_devm_platform_get_and_ioremap_resource.cocci”
>> +@display depends on context@ >> +expression base, device1, device2, index, private, resource; >> +@@ >> +( >> +*resource = platform_get_resource(device1, IORESOURCE_MEM, index); >> + base = >> +* devm_ioremap_resource >> + (>dev, resource); > > Why do you require these statements to be next to each other? I would appreciate indications for a general change acceptance also according to such a simple transformation approach. I imagine that it will become more challenging to tolerate extra source code between these function calls (by the specification of special SmPL filters). >> +| >> +*private->res = platform_get_resource(device1, IORESOURCE_MEM, index); >> + base = >> +* devm_ioremap_resource >> + (device2, private->res); > > Why do you have this special case? The usage of the data structure member “res” triggers corresponding software design consequences. The expressions which are passed as the first function call parameters can be different. >> +@replacement depends on patch@ >> +expression base, device1, device2, index, private, resource; >> +@@ >> +( >> +-resource = platform_get_resource(device1, IORESOURCE_MEM, index); >> + base = >> +- devm_ioremap_resource >> ++ devm_platform_get_and_ioremap_resource >> + ( >> +- & >> + device1 >> +- ->dev >> + , >> +- resource >> ++ index, >> + ); >> +| >> +-private->res = platform_get_resource(device1, IORESOURCE_MEM, index); >> + base = >> +- devm_ioremap_resource >> ++ devm_platform_get_and_ioremap_resource >> + (device2, > > It is very suspicious that in one case you change the first argument of > devm_platform_get_and_ioremap_resource and in one case you don't. I noticed a few special cases during my source code analysis approach. > If you don't know how to make the change in some cases, it would be better > to do nothing at all. How do you think about to take another look at any update candidates? Examples: * mvebu_sei_probe https://elixir.bootlin.com/linux/v5.9-rc4/source/drivers/irqchip/irq-mvebu-sei.c#L368 https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/irqchip/irq-mvebu-sei.c?id=f4d51dffc6c01a9e94650d95ce0104964f8ae822#n368 * hi655x_pmic_probe https://elixir.bootlin.com/linux/v5.9-rc4/source/drivers/mfd/hi655x-pmic.c#L92 https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/mfd/hi655x-pmic.c?id=f4d51dffc6c01a9e94650d95ce0104964f8ae822#n92 Regards, Markus ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [PATCH] Coccinelle: api: Add SmPL script “use_devm_platform_get_and_ioremap_resource.cocci”
From: Markus Elfring
Date: Mon, 7 Sep 2020 13:14:44 +0200
Another wrapper function is available since the commit
890cc39a879906b63912482dfc41944579df2dc6
("drivers: provide devm_platform_get_and_ioremap_resource()").
Provide design options for the adjustment of affected source code
by the means of the semantic patch language (Coccinelle software).
Signed-off-by: Markus Elfring
---
...vm_platform_get_and_ioremap_resource.cocci | 71 +++
1 file changed, 71 insertions(+)
create mode 100644
scripts/coccinelle/api/use_devm_platform_get_and_ioremap_resource.cocci
diff --git
a/scripts/coccinelle/api/use_devm_platform_get_and_ioremap_resource.cocci
b/scripts/coccinelle/api/use_devm_platform_get_and_ioremap_resource.cocci
new file mode 100644
index ..8e67359f6b76
--- /dev/null
+++ b/scripts/coccinelle/api/use_devm_platform_get_and_ioremap_resource.cocci
@@ -0,0 +1,71 @@
+// SPDX-License-Identifier: GPL-2.0
+/// Simplify a function call combination by using a known wrapper function.
+//
+// Keywords: wrapper function conversion ioremap resources
+// Confidence: High
+
+virtual context, patch, report, org
+
+@display depends on context@
+expression base, device1, device2, index, private, resource;
+@@
+(
+*resource = platform_get_resource(device1, IORESOURCE_MEM, index);
+ base =
+* devm_ioremap_resource
+ (>dev, resource);
+|
+*private->res = platform_get_resource(device1, IORESOURCE_MEM, index);
+ base =
+* devm_ioremap_resource
+ (device2, private->res);
+)
+
+@replacement depends on patch@
+expression base, device1, device2, index, private, resource;
+@@
+(
+-resource = platform_get_resource(device1, IORESOURCE_MEM, index);
+ base =
+- devm_ioremap_resource
++ devm_platform_get_and_ioremap_resource
+ (
+- &
+ device1
+- ->dev
+ ,
+- resource
++ index,
+ );
+|
+-private->res = platform_get_resource(device1, IORESOURCE_MEM, index);
+ base =
+- devm_ioremap_resource
++ devm_platform_get_and_ioremap_resource
+ (device2,
+- private->res
++ index, >res
+ );
+)
+
+@or depends on org || report@
+expression base, device1, device2, index, private, resource;
+position p;
+@@
+(
+ resource = platform_get_resource(device1, IORESOURCE_MEM, index);
+ base = devm_ioremap_resource@p(>dev, resource);
+|
+ private->res = platform_get_resource(device1, IORESOURCE_MEM, index);
+ base = devm_ioremap_resource@p(device2, private->res);
+)
+
+@script:python to_do depends on org@
+p << or.p;
+@@
+coccilib.org.print_todo(p[0], "WARNING: opportunity for
devm_platform_get_and_ioremap_resource()")
+
+@script:python reporting depends on report@
+p << or.p;
+@@
+coccilib.report.print_report(p[0], "WARNING: opportunity for
devm_platform_get_and_ioremap_resource()")
--
2.28.0
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH] coccinelle: ifnullfree: add vfree(), kvfree*() functions
> Extend the list of free functions with kvfree(), kvfree_sensitive(), > vfree(). How do you think about further software extensions for this source code search pattern? Another example for a corresponding advanced data processing approach: https://build.opensuse.org/project/show/home:elfring:semantic_patching:Deletion_of_checks_before_specific_function_calls Regards, Markus ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] transform oddity
> Is it me not understanding cocci grammar again?
You are also struggling with expressing your data processing needs
by the means of the semantic patch language.
You would like to rename three parameters for selected functions.
I imagine that an other transformation specification will be more appropriate
for the function body.
> {
> ...
> (
> - arg1
> + dev
> |
> - arg2
> + attr
> |
> - arg3
> + buf
> )
> ... when any
> }
I suggest to use a SmPL nest construct.
https://github.com/coccinelle/coccinelle/blob/730dbb034559b3e549ec0b2973cd0400a3fa072f/docs/manual/cocci_syntax.tex#L789
{
<+...
(
-arg1
+dev
|
-arg2
+attr
|
-arg3
+buf
)
...+>
}
> identifier d_show =~ "^.*show.*$";
By the way:
Would you like to omit the specification “.*$” from the regular expression
for this SmPL constraint?
Regards,
Markus
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [RFC PATCH] coccinelle: api: add flex_array_size.cocci script
> Suggest flex_array_size() wrapper to compute the size of a
> flexible array member in a structure. The macro additionally
> checks for integer overflows.
Can the following script variant for the semantic patch language help
to clarify any software development ideas and remaining open issues?
virtual context, patch, report, org
@decl_flex@
identifier name, array, size;
type TA, TS;
@@
struct name {
...
TS size;
...
(TA array[];
|TA array[ \( 0 \| 1 \) ];
)
};
@ptr_flex@
identifier decl_flex.name, instance;
@@
struct name *instance;
@struct_flex@
identifier decl_flex.name, instance;
@@
struct name instance;
@ptr_flex_size depends on !patch@
identifier decl_flex.array, decl_flex.size, ptr_flex.instance;
type decl_flex.TA;
position p;
@@
*instance->size *@p \( sizeof(TA) \| sizeof(*instance->array) \)
@depends on patch exists@
identifier decl_flex.array, decl_flex.size, ptr_flex.instance;
type decl_flex.TA;
@@
(
-sizeof(TA)
|
-sizeof(*instance->array)
)
- *
+flex_array_size(instance, array,
instance->size
+)
@struct_flex_size depends on !patch@
identifier decl_flex.array, decl_flex.size, struct_flex.instance;
type decl_flex.TA;
position p;
@@
*instance->size *@p \( sizeof(TA) \| sizeof(*instance->array) \)
@depends on patch exists@
identifier decl_flex.array, decl_flex.size, struct_flex.instance;
type decl_flex.TA;
@@
(
-sizeof(TA)
|
-sizeof(*instance->array)
)
- *
+flex_array_size(instance, array,
instance->size
+)
@func_arg_flex_size depends on !patch@
identifier decl_flex.name, decl_flex.array, decl_flex.size, func, instance;
type decl_flex.TA;
position p;
@@
func(..., struct name *instance, ...) {
... when any
*instance->size *@p \( sizeof(TA) \| sizeof(*instance->array) \)
...
}
@depends on patch exists@
identifier decl_flex.name, decl_flex.array, decl_flex.size, func, instance;
type decl_flex.TA;
@@
func(..., struct name *instance, ...) {
... when any
(
-sizeof(TA)
|
-sizeof(*instance->array)
)
- *
+flex_array_size(instance, array,
instance->size
+)
...
}
@script:python depends on report@
p << ptr_flex_size.p;
@@
coccilib.report.print_report(p[0], "WARNING opportunity for flex_array_size")
@script:python depends on org@
p << ptr_flex_size.p;
@@
coccilib.org.print_todo(p[0], "WARNING opportunity for flex_array_size")
@script:python depends on report@
p << struct_flex_size.p;
@@
coccilib.report.print_report(p[0], "WARNING opportunity for flex_array_size")
@script:python depends on org@
p << struct_flex_size.p;
@@
coccilib.org.print_todo(p[0], "WARNING opportunity for flex_array_size")
@script:python depends on report@
p << func_arg_flex_size.p;
@@
coccilib.report.print_report(p[0], "WARNING opportunity for flex_array_size")
@script:python depends on org@
p << func_arg_flex_size.p;
@@
coccilib.org.print_todo(p[0], "WARNING opportunity for flex_array_size")
Regards,
Markus
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Checking the application of a SmPL disjunction for a function call addition
>> Will this information trigger further consequences for the clarification >> of the topic “[RFC PATCH] coccinelle: api: add flex_array_size.cocci script”? >> https://lore.kernel.org/cocci/20200828163134.496386-1-efre...@linux.com/ >> https://systeme.lip6.fr/pipermail/cocci/2020-August/008169.html > > As far as I can see, in that semantic patch, array is always inherited > from a previous rule. Does this technical detail change the handling of the discussed SmPL disjunction in any ways? Regards, Markus ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Checking the application of a SmPL disjunction for a function call addition
>> @replacement@ >> identifier array, instance, size; >> type T; >> @@ >> ( >> -sizeof(T) >> | >> -sizeof(*instance->array) >> ) >> * >> +flex_array_size(instance, array, >> instance->size >> + ) > > This semantic patch will fail if the sizeof(T) option is matched, becuse > then it won't be able to create the + code, since it won't know what array > should be. Will this information trigger further consequences for the clarification of the topic “[RFC PATCH] coccinelle: api: add flex_array_size.cocci script”? https://lore.kernel.org/cocci/20200828163134.496386-1-efre...@linux.com/ https://systeme.lip6.fr/pipermail/cocci/2020-August/008169.html Regards, Markus ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] Checking the deletion of a multiplication operator with SmPL
Hello, The following small script variant for the semantic patch language gets accepted according to the software combination “Coccinelle 1.0.8-00159-g730dbb03”. @replacement@ identifier array, instance, size; type T; @@ ( -sizeof(T) | -sizeof(*instance->array) ) * +flex_array_size(instance, array, instance->size + ) But I observe data processing difficulties if I would like to mark the multiplication operator also for deletion (on a separate line). elfring@Sonne:~/Projekte/Coccinelle/Probe> spatch --parse-cocci flex_array_size-test2-20200829.cocci init_defs_builtins: /usr/local/bin/../lib/coccinelle/standard.h File "flex_array_size-test2-20200829.cocci", line 10, column 1, charpos = 103 around = '*', whole content = -* Can such a transformation specification become supported? Regards, Markus ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] Checking the usage of a SmPL disjunction within a sizeof()
Hello, I have tried another tiny script variant out for the semantic patch language (according to the software combination “Coccinelle 1.0.8-00159-g730dbb03”). @display@ identifier array, instance; type T; @@ *sizeof( \( T \| *(instance->array) \) ) elfring@Sonne:~/Projekte/Coccinelle/Probe> spatch --parse-cocci sizeof-test-20200828.cocci … minus: parse error: File "sizeof-test-20200828.cocci", line 5, column 17, charpos = 66 around = '*', whole content = *sizeof( \( T \| *(instance->array) \) ) May I expect that such a SmPL disjunction should also work? Regards, Markus ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] coccinelle: Convert comma to semicolon
> > Add an imperfect test to detect these comma uses. > > > > No false positives were found in testing, but many types of false negatives > > are possible. > > > > e.g.: > > foo = bar() + 1,/* comma use, but not direct assignment */ > > bar = baz(); > > Hi. > > I recently added a test for this condition to linux's checkpatch. > > A similar coccinelle script might be: I find it interesting that you present another transformation approach for the semantic patch language. > $ cat comma.cocci > @@ > expression e1; > expression e2; > @@ > > e1 > - , > + ; > e2; Such a tiny SmPL script looks so simple. > This works reasonably well but it has several false positives > for declarations like: Would you like to filter the usage of code like “LIST_HEAD(list)” out? Are there any more software development challenges to consider for special assignment statements? Regards, Markus ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] coccinelle: api: add sprintf() support to device_attr_show
> The problem has nothing to do with disjunctions. Can missing source code matches trigger the consequence that questionable branches would be applied there? Will the clarification for the issue “failing tests - TODO” be continued? Regards, Markus ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] coccinelle: api: add sprintf() support to device_attr_show
> I will see if it can be fixed. How will the issue “failing tests - TODO” evolve further? https://github.com/coccinelle/coccinelle/commit/f2d7ec9006c89610bd1aab4662fcf100e3e6d469#diff-13ff769079511ec7b5dddef7143b2b93R1 How do the comments there fit to undesirable effects for SmPL disjunctions? Regards, Markus ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Searching for format strings with SmPL disjunctions
How do you think about to extend a source code search pattern like the following
by using a regular expression as a SmPL constraint?
@display@
format F =~ "l{0,2}[diuxX]";
@@
*\("%@F@" \| "%@F@\n"\)
Regards,
Markus
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Searching for format strings with SmPL disjunctions
> There are no short term plans for this. Can long term development considerations be influenced for advanced data processing of format strings? Regards, Markus ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Searching for format strings with SmPL disjunctions
Can a source code search pattern like the following become ever usable?
@display@
@@
*"%"
("d"
|"ld"
|"lld"
)
("\n"
|""
)
elfring@Sonne:~/Projekte/Coccinelle/Probe> spatch --parse-cocci
show_usage_of_selected_format_strings.cocci
…
minus: parse error:
File "show_usage_of_selected_format_strings.cocci", line 4, column 0, charpos
= 18
around = '(',
whole content = ("d"
Regards,
Markus
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [PATCH v2] drm/nouveau/gem: Use vmemdup_user() rather than duplicating its implementation
From: Markus Elfring
Date: Fri, 14 Aug 2020 08:56:54 +0200
* Reuse existing functionality from vmemdup_user() instead of keeping
duplicate source code.
Generated by: scripts/coccinelle/api/memdup_user.cocci
* See also:
[PATCH] drm/nouveau/gem: fix err_cast.cocci warnings
* Simplify this function implementation further by omitting the local
variable “mem” and extra error handling here.
Reported-by: kernel test robot
Signed-off-by: Markus Elfring
---
drivers/gpu/drm/nouveau/nouveau_gem.c | 13 +
1 file changed, 1 insertion(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_gem.c
b/drivers/gpu/drm/nouveau/nouveau_gem.c
index 81f111ad3f4f..536ad5e2cbe6 100644
--- a/drivers/gpu/drm/nouveau/nouveau_gem.c
+++ b/drivers/gpu/drm/nouveau/nouveau_gem.c
@@ -583,21 +583,10 @@ u_free(void *addr)
static inline void *
u_memcpya(uint64_t user, unsigned nmemb, unsigned size)
{
- void *mem;
void __user *userptr = (void __force __user *)(uintptr_t)user;
size *= nmemb;
-
- mem = kvmalloc(size, GFP_KERNEL);
- if (!mem)
- return ERR_PTR(-ENOMEM);
-
- if (copy_from_user(mem, userptr, size)) {
- u_free(mem);
- return ERR_PTR(-EFAULT);
- }
-
- return mem;
+ return vmemdup_user(userptr, size);
}
static int
--
2.28.0
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH v3] documentation: coccinelle: Improve command example for make C={1, 2}
> the usage of the makefile C variable flag by coccicheck. * Can it be confusing to denote an item as a variable and a flag? * Would you really like to stress here that a flag can be variable? > +This variable can be used to run scripts for … Can the scope for a make command be selected also without such a variable? Will clarification requests for previously mentioned background information influence the proposed descriptions any further? Regards, Markus ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [PATCH] Coccinelle: Reduce duplicate code for patch rules of memdup_user.cocci
From: Markus Elfring
Date: Sun, 9 Aug 2020 11:11:20 +0200
Another patch rule was added. A bit of code was copied from a previous
SmPL rule for this change specification.
* Thus reduce duplicate code by using another SmPL disjunction.
* Increase the precision a bit for desired source code adjustments.
Fixes: 9c568dbd677bcfc975220d3157c89c48669a23e3 ("coccinelle: api: extend
memdup_user rule with vmemdup_user()")
Signed-off-by: Markus Elfring
---
scripts/coccinelle/api/memdup_user.cocci | 44 +---
1 file changed, 16 insertions(+), 28 deletions(-)
diff --git a/scripts/coccinelle/api/memdup_user.cocci
b/scripts/coccinelle/api/memdup_user.cocci
index e01e95108405..7cf698b4e925 100644
--- a/scripts/coccinelle/api/memdup_user.cocci
+++ b/scripts/coccinelle/api/memdup_user.cocci
@@ -27,34 +27,22 @@ expression from,to,size;
identifier l1,l2;
position p : script:python() { relevant(p) };
@@
-
-- to = \(kmalloc@p\|kzalloc@p\)
-- (size,\(GFP_KERNEL\|GFP_USER\|
--\(GFP_KERNEL\|GFP_USER\)|__GFP_NOWARN\));
-+ to = memdup_user(from,size);
- if (
-- to==NULL
-+ IS_ERR(to)
- || ...) {
- <+... when != goto l1;
-- -ENOMEM
-+ PTR_ERR(to)
- ...+>
- }
-- if (copy_from_user(to, from, size) != 0) {
--<+... when != goto l2;
---EFAULT
--...+>
-- }
-
-@depends on patch@
-expression from,to,size;
-identifier l1,l2;
-position p : script:python() { relevant(p) };
-@@
-
-- to = \(kvmalloc@p\|kvzalloc@p\)(size,\(GFP_KERNEL\|GFP_USER\));
-+ to = vmemdup_user(from,size);
+ to =
+(
+- \(kmalloc@p\|kzalloc@p\)
++ memdup_user
+ (
+- size, \( \(GFP_KERNEL\|GFP_USER\) \|
\(GFP_KERNEL\|GFP_USER\)|__GFP_NOWARN \)
++ from, size
+ )
+|
+- \(kvmalloc@p\|kvzalloc@p\)
++ vmemdup_user
+ (
+- size, \(GFP_KERNEL\|GFP_USER\)
++ from, size
+ )
+);
if (
- to==NULL
+ IS_ERR(to)
--
2.28.0
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH v2] documentation: coccinelle: Improve command example for make C={1, 2}
> Modify coccinelle documentation to further clarify
> the usage of the makefile C variable flag by coccicheck.
How do you think about a wording variant like the following
for the change description?
Clarify the usage of the make variable “C” for coccicheck.
> +C flag is used. The C flag is a variable used by the makefile
Can such a wording approach trigger understanding difficulties?
> +This flag can be used to
> +run scripts for …
I find this description improvable.
> +The value 1 is passed to the C flag to check for files that make considers
> +need to be recompiled.::
Would you like to distinguish consequences between “compilation” and
“recompilation”?
The mentioned parameter is assigned to the macro “KBUILD_CHECKSRC”.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Makefile?id=fc80c51fd4b23ec007e88d4c688f2cac1b8648e7#n198
The macro “KBUILD_CHECKSRC” is eventually checked for the setting of a few
additional macros.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/scripts/Makefile.build?id=fc80c51fd4b23ec007e88d4c688f2cac1b8648e7#n97
Would you like to determine where these macros are actually applied?
Do you find the commit 0c33f125732d0d33392ba6774d85469d565d3496 ("kbuild:
run the checker after the compiler") from 2020-07-07 interesting for
the discussed documentation adjustment?
https://lore.kernel.org/patchwork/patch/1260832/
https://lore.kernel.org/lkml/20200622154512.82758-1-luc.vanoostenr...@gmail.com/
> +make C=1 CHECK="scripts/coccicheck" "drivers/bluetooth/bfusb.o"
* Can double quotes be omitted here?
* How do you think about to enclose any data by apostrophes?
Regards,
Markus
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH 42/43] tests: Add test case to match meta attribute
… > +++ b/tests/metaattr.cocci > @@ -0,0 +1,9 @@ > +@@ > +attribute name __attr__; > +attribute a; > +identifier b; > +@@ > + > +-int > ++char > + b a = 1; I have got understanding difficulties for this test SmPL script. I interpret such SmPL code in the way that the metavariable “__attr__” is declared while it is not used further. Will the distinction between the metavariable types “attribute” and “attribute name” trigger any extensions for the software documentation? Regards, Markus ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH 42/43] tests: Add test case to match meta attribute
…
> +++ b/tests/metaattr.c
> @@ -0,0 +1,5 @@
> +int main() {
> + int b __attr__ = 1;
> + int b = 1;
> + return 0;
> +}
* Should such test code really work with a repeated definition of
the variable “b”?
* Would you like to test here if the identifier “b” should be handled
as an attribute for the variable “__attr__”?
Regards,
Markus
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [PATCH] Coccinelle: Add a SmPL script for the reconsideration of function calls before return statements
From: Markus Elfring
Date: Wed, 8 Jul 2020 19:09:59 +0200
It can happen that function implementations with the return type “void”
call a special function at the end of if branches.
Such calls are occasionally followed by an immediate return.
The same function can be called at the end of the function implementation.
Thus it can be helpful to replace previous function calls
by goto statements.
Provide design options for the adjustment of affected source code
by the means of the semantic patch language (Coccinelle software).
Signed-off-by: Markus Elfring
---
.../misc/goto_last_function_call.cocci| 89 +++
1 file changed, 89 insertions(+)
create mode 100644 scripts/coccinelle/misc/goto_last_function_call.cocci
diff --git a/scripts/coccinelle/misc/goto_last_function_call.cocci
b/scripts/coccinelle/misc/goto_last_function_call.cocci
new file mode 100644
index ..92dcf3626c48
--- /dev/null
+++ b/scripts/coccinelle/misc/goto_last_function_call.cocci
@@ -0,0 +1,89 @@
+// SPDX-License-Identifier: GPL-2.0
+/// Reconsider function calls before a return statement in if branches.
+//
+// Keywords: duplicate function calls common exception handling
+// Confidence: Low
+// See also:
+// Clarification request “Adding labels without indentation before specific
statements?”
+// https://lore.kernel.org/cocci/4b3bb651-5db0-021c-cbea-347eda0e9...@web.de/
+
+virtual context, patch, report, org
+
+@display depends on context@
+expression action;
+expression list el;
+identifier work;
+@@
+ void work(...)
+ {
+ <+...
+ if (...)
+ {
+...
+* action(el);
+* return;
+ }
+ ...+>
+*action(el);
+ }
+
+@replacement depends on patch@
+expression action, condition;
+expression list el;
+identifier work;
+@@
+ void work(...)
+ {
+ <+...
+(
+-if (condition)
+-{
+- action(el);
+- return;
+-}
++if (condition)
++ goto last_action;
+|
+ if (...)
+ {
+...
+- action(el);
+- return;
++ goto last_action;
+ }
+)
+ ...+>
++last_action:
+ action(el);
+ }
+
+@or depends on org || report@
+expression action;
+expression list el;
+identifier work;
+position p;
+@@
+ void work(...)
+ {
+ <+...
+ if (...)
+ {
+...
+action(el);
+return;
+ }
+ ...+>
+ action@p(el);
+ }
+
+@script:python to_do depends on org@
+p << or.p;
+@@
+coccilib.org.print_todo(p[0],
+"WARNING: The same function was called at the end of
an if branch before. Would you like to avoid duplicate function calls?")
+
+@script:python reporting depends on report@
+p << or.p;
+@@
+coccilib.report.print_report(p[0],
+ "WARNING: The same function was called at the end
of an if branch before. Would you like to avoid duplicate function calls?")
--
2.27.0
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] Adding labels without indentation before specific statements?
Hello,
I have tried another tiny script variant out for the semantic patch language
(according to the software combination “Coccinelle 1.0.8-00139-gd0fd4c7d”).
@addition@
identifier work;
expression action;
@@
void work(...)
{
... when any
+last_action:
action(...);
}
Such a simple adjustment approach can work as expected (in principle).
It seems that indentation is also applied to the added label according to
the function call at the end of found function implementations.
But the Linux coding style prefers to start labels in the first column.
How should patches be accordingly generated then?
Regards,
Markus
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] Checking data processing results for a special SmPL disjunction
Hello,
I have tried another tiny script variant out for the semantic patch language
(according to the software combination “Coccinelle 1.0.8-00131-g675b9670”).
@display@
expression* pointer;
identifier result;
type t;
@@
t result = <+...
( sizeof(*(pointer))
|
* *(pointer)
) ...+>;
elfring@Sonne:~/Projekte/Linux/next-patched> spatch
~/Projekte/Coccinelle/janitor/show_interesting_disjunction-20200627.cocci
drivers/base/regmap/regmap.c
…
@@ -1263,7 +1261,6 @@ EXPORT_SYMBOL_GPL(devm_regmap_field_free
struct regmap_field *regmap_field_alloc(struct regmap *regmap,
struct reg_field reg_field)
{
- struct regmap_field *rm_field = kzalloc(sizeof(*rm_field), GFP_KERNEL);
if (!rm_field)
return ERR_PTR(-ENOMEM);
Now I wonder why such a diff hunk is generated.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/base/regmap/regmap.c?id=95b2c3ec4cb1689db2389c251d39f64490ba641c#n1254
https://elixir.bootlin.com/linux/v5.7.2/source/drivers/base/regmap/regmap.c#L1261
I expected that the first element in the shown SmPL disjunction
should exclude displays for the usage of the operator “sizeof”.
Is my understanding still incomplete also for this software behaviour?
Regards,
Markus
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Checking the parsing for a nested SmPL disjunction
I assumed that I may omit the semicolon in such a SmPL code. Can the specification of a SmPL nest construct ever be sufficient here? >>> >>> No. <+... ...+> matches a subtree of an AST. For a variable >>> initialization, there is no subtree of the AST that includes both the >>> right side of an = and the ;. >> >> Can the abstract syntax tree be adjusted accordingly? > > No. Can such a rejection mean that software development efforts look undesirable (while adjustments for the data model would eventually be possible)? Regards, Markus ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Checking the parsing for a nested SmPL disjunction
>> I assumed that I may omit the semicolon in such a SmPL code. >> Can the specification of a SmPL nest construct ever be sufficient here? > > No. <+... ...+> matches a subtree of an AST. For a variable > initialization, there is no subtree of the AST that includes both the > right side of an = and the ;. Can the abstract syntax tree be adjusted accordingly? Regards, Markus ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Checking the parsing for a nested SmPL disjunction
>> <+... when any >> (t2 y = <+... >> ( sizeof(*(resource)) >> | >> * *(resource) >> ) ...+> > > You are missing a ; here. I assumed that I may omit the semicolon in such a SmPL code. Can the specification of a SmPL nest construct ever be sufficient here? Regards, Markus ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] Checking the parsing for a nested SmPL disjunction
Hello, I have tried another small script variant out for the semantic patch language (according to the software combination “Coccinelle 1.0.8-00131-g675b9670”). @display@ type t1, t2; expression action; identifier resource, y; statement is, es; @@ t1* resource; <+... when any (t2 y = <+... ( sizeof(*(resource)) | * *(resource) ) ...+> | *action(..., resource, ...) ) ...+> elfring@Sonne:~/Projekte/Coccinelle/janitor> spatch --parse-cocci show_problematic_disjunction-20200626.cocci … minus: parse error: File "show_problematic_disjunction-20200626.cocci", line 14, column 0, charpos = 196 around = '|', whole content = | May I expect that such SmPL disjunctions should also work? Regards, Markus ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] Checking the display of SmPL isomorphism results (with pointers)
Hello, I have tried another tiny script variant out for the semantic patch language (according to the software combination “Coccinelle 1.0.8-00131-g675b9670”). @display@ expression* x; statement is, es; @@ *if (!x) is else es elfring@Sonne:~/Projekte/Coccinelle/janitor> spatch --parse-cocci show_null_pointer_checks.cocci … if (*!*x *!= *NULL) … if (*!*NULL *!= *x) … if (*x *!= *NULL *== *NULL) … if (*NULL *!= *x *== *NULL) … if (*NULL *== *x *!= *NULL) … if (*NULL *== *NULL *!= *x) … Now I find the shown lines in such a program output questionable. Should these presentations of elements in SmPL disjunctions be shorter? Regards, Markus ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] Checking the parsing for a specific SmPL disjunction
Hello, I know that the following SmPL script example gets accepted by the Coccinelle software. @display@ expression x, y; @@ *y = ... *(x) ... I have tried another tiny script variant out for the semantic patch language (according to the software combination “Coccinelle 1.0.8-00131-g675b9670”). @display@ expression action, x, y; @@ ( *y = ... *(x) ... | *action(..., x, ...) ) elfring@Sonne:~/Projekte/Coccinelle/janitor> spatch --parse-cocci show_pointer_dereferences3.cocci init_defs_builtins: /usr/local/bin/../lib/coccinelle/standard.h minus: parse error: File "show_pointer_dereferences3.cocci", line 5, column 14, charpos = 54 around = '...', whole content = *y = ... *(x) ... Should such SmPL disjunctions work in the future? Regards, Markus ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Checking the replacement of two specific function calls
elfring@Sonne:~/Projekte/Linux/next-patched> spatch --include-headers-for-types ~/Projekte/Coccinelle/janitor/use_devm_platform_get_and_ioremap_resource3.cocci drivers/i2c/busses/i2c-rcar.c >>> >>> No, include headers for types doesn't have any impact on how many header >>> files are included. You need options like --all-includes or >>> --recursive-includes. >> >> I still observe that the known patch hunk is not generated even with the >> addition >> of one of these command options. >> https://lore.kernel.org/patchwork/patch/1223734/ >> https://lore.kernel.org/linux-i2c/20200414134827.18674-1-zhengdej...@gmail.com/ >> >> Would I need any extra parameters here? > > Perhaps -I options to help it find the relevant .h file. I became curious if the software situation can be clarified better. Thus I have tested my transformation approach again with the commit “i2c: altera: use proper variable to hold errno” before the referenced change. @replacement@ expression base, device, resource; @@ -resource = platform_get_resource(device, IORESOURCE_MEM, 0); base = - devm_ioremap_resource(>dev, resource) + devm_platform_get_and_ioremap_resource(device, 0, ) ; elfring@Sonne:~/Projekte/Linux/next-patched> git checkout edb2c9dd3948738ef030c32b948543e84f4d3f81 && LANG=C make MODE=patch COCCI=~/Projekte/Coccinelle/janitor/use_devm_platform_get_and_ioremap_resource3.cocci M=drivers/i2c/busses V=1 coccicheck Running (4 in parallel): /usr/local/bin/spatch -D patch --very-quiet --cocci-file /home/elfring/Projekte/Coccinelle/janitor/use_devm_platform_get_and_ioremap_resource3.cocci --patch . --dir drivers/i2c/busses -I ./arch/x86/include -I ./arch/x86/include/generated -I ./include -I ./arch/x86/include/uapi -I ./arch/x86/include/generated/uapi -I ./include/uapi -I ./include/generated/uapi --include ./include/linux/kconfig.h --jobs 4 --chunksize 1 coccicheck failed make: *** [Makefile:1781: coccicheck] Error 255 I can try another customised command out with the shown parameters. elfring@Sonne:~/Projekte/Linux/next-patched> spatch -I ./arch/x86/include -I ./arch/x86/include/generated -I ./include -I ./arch/x86/include/uapi -I ./arch/x86/include/generated/uapi -I ./include/uapi -I ./include/generated/uapi --include ./include/linux/kconfig.h ~/Projekte/Coccinelle/janitor/use_devm_platform_get_and_ioremap_resource3.cocci drivers/i2c/busses/i2c-rcar.c init_defs_builtins: /usr/local/bin/../lib/coccinelle/standard.h HANDLING: drivers/i2c/busses/i2c-rcar.c May I expect the generation of a patch by the Coccinelle software here? Regards, Markus ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH] coccinelle: misc: add array_size_dup script to detect missed overflow checks
> ( > * size = E1 * E2;@p > | > * size = E1 * E2 * E3;@p > | > * size = E1 * E2 + E3;@p > ) I suggest to reconsider also the order of elements for such a SmPL disjunction. Can a computation like “E2 * E3” also be matched by the expression metavariable “E2” alone? Regards, Markus ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH] coccinelle: misc: Add array_size_dup script to detect missed overflow checks
I propose once more to avoid a typo in the previous patch subject. … > ( > - size = E1 * E2; > + size = array_size(E1, E2); > | > - size = E1 * E2 * E3; > + size = array3_size(E1, E2, E3); > | > - size = E1 * E2 + E3; > + size = struct_size(E1, E2, E3); > ) How do you think about to use SmPL disjunctions like the following? size = ( - (E1) * (E2) + array_size(E1, E2) | - (E1) * (E2) * (E3) + array3_size(E1, E2, E3) | - (E1) * (E2) + (E3) + struct_size(E1, E2, E3) ); > ... when != size = E4 > when != size += E4 … Can it become helpful to express a constraint for a metavariable of the type “assignment operator”? > ( > * size = E1 * E2;@p > | > * size = E1 * E2 * E3;@p > | > * size = E1 * E2 + E3;@p > ) *size =@p \( (E1) * (E2) \| (E1) * (E2) * (E3) \| (E1) * (E2) + (E3) \) Regards, Markus ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Software analysis around data type annotations
>> I am going to present similar questions occasionally. > > Please stop. I hope that I can continue as usual for while according to a desire to achieve further improvements. > The chance that people will help you will increase. This would be nice. > No one is interested in quantifying software development resources. Really? I suggest to reconsider such a view. > Even your favorite phrase "my software development attention" > is really really annoying. Just drop "software development" > from your vocabulary and it will be a step in the right direction. Will it become more interesting to clarify corresponding communication difficulties? Regards, Markus ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Software analysis around data type annotations
> *void * __iomem action(...)
> {...}
>
> It's not quite in line with the what-you-see-is-what-you-get principle,
> but it'll work for your use case.
Usable output is produced after the adjusted position for the macro.
elfring@Sonne:~/Projekte/Linux/next-patched> spatch
~/Projekte/Coccinelle/janitor/show_iomem_functions3.cocci
drivers/base/platform.c
…
-void __iomem *
-devm_platform_ioremap_resource_byname(struct platform_device *pdev,
- const char *name)
…
>> How many software development resources will be needed
>> to get such SmPL script variants to work?
>
> Please stop asking such questions
I am going to present similar questions occasionally.
> and I'll be happy to help you out.
Thanks for your positive feedback.
I imagine that I can offer more concrete development items
(besides the ones you might know already) if I would get
also additional resources.
Regards,
Markus
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Software analysis around data type annotations
> I'm away from my computer right now so can't test this, but try declaring
> __iomem as an attribute:
@display@
attribute name __iomem;
identifier action;
@@
*void __iomem * action(...)
{ ... }
elfring@Sonne:~/Projekte/Coccinelle/janitor> spatch --parse-cocci
show_iomem_functions2.cocci
…
minus: parse error:
File "show_iomem_functions2.cocci", line 5, column 14, charpos = 70
around = '*',
whole content = *void __iomem * action(...)
Test version: 1.0.8-00131-g675b9670
How many software development resources will be needed
to get such SmPL script variants to work?
Regards,
Markus
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] Software analysis around data type annotations
Hello,
The support for data processing with attributes was extended recently.
https://github.com/coccinelle/coccinelle/commits?q=committer-date%3A%3C2020-06-16
Under which circumstances will a source code analysis approach become supported
by a script (like the following) for the semantic patch language?
@display@
identifier action;
@@
*void __iomem * action(...)
{ ... }
elfring@Sonne:~/Projekte/Coccinelle/janitor> spatch --parse-cocci
show_iomem_functions.cocci
…
minus: parse error:
File "show_iomem_functions.cocci", line 4, column 14, charpos = 46
around = '*',
whole content = *void __iomem * action(...)
Regards,
Markus
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] coccinelle: api: add device_attr_show script
>> +virtual report, org, context, patch
>>
>> Is such a SmPL code variant more succinct?
>
> This doens't matter.
Can less duplicate code be a bit nicer?
>>> +ssize_t show(struct device *dev, struct device_attribute *attr, char *buf)
>>> +{
>>> + <...
>>> +* return snprintf@p(...);
>>> + ...>
>>> +}
>>
>> I suggest to reconsider the selection of the SmPL nest construct.
>> https://github.com/coccinelle/coccinelle/blob/e06b9156dfa02a28cf3cbf0913a10513f3d163ab/docs/manual/cocci_syntax.tex#L783
>>
>> Can the construct “<+... … ...+>” become relevant here?
>
> <... ...> is fine if the only thing that will be used afterwards is what
> is inside the <... ...>
I propose once more to distinguish better if the shown return statement
may be really treated as optional for such a source code search approach
(or not).
Regards,
Markus
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH] coccinelle: api: add device_attr_show script
> +// Confidence: High
Would you like to add any suggestion for a possible patch message?
…
> +virtual report
> +virtual org
> +virtual context
> +virtual patch
+virtual report, org, context, patch
Is such a SmPL code variant more succinct?
…
> +ssize_t show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> + <...
> +*return snprintf@p(...);
> + ...>
> +}
I suggest to reconsider the selection of the SmPL nest construct.
https://github.com/coccinelle/coccinelle/blob/e06b9156dfa02a28cf3cbf0913a10513f3d163ab/docs/manual/cocci_syntax.tex#L783
Can the construct “<+... … ...+>” become relevant here?
Would you like to consider any further software design consequences
around the safe application of the asterisk functionality in rules
for the semantic patch language?
Regards,
Markus
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH v3] coccinelle: api: add kzfree script
… > +virtual context > +virtual patch > +virtual org > +virtual report +virtual context, patch, org, report Is such a SmPL code variant more succinct? … > +if (...) > + \(memset@ok\|memzero_explicit@ok\)(...); Would you like to tolerate any extra source code around such a function call in an if branch? … > +( > +* memset@m((T)E, 0, ...); > +| > +* memzero_explicit@m((T)E, ...); > +) … I suggest to move a semicolon. +( +*memset@m((T)E, 0, ...) +| +*memzero_explicit@m((T)E, ...) +); … > +- \(kfree\|vfree\|kvfree\)(E); > ++ kvfree_sensitive(E, size); … Would you like to increase the precision a bit for the change specification? +-\(kfree\|vfree\|kvfree\) ++kvfree_sensitive + (E ++ , size + ); … > +( > +- kfree(E); > ++ kzfree(E); > +| > +- \(vfree\|kvfree\)(E); > ++ kvfree_sensitive(E, size); > +) … +( +-kfree ++kzfree + (E) +| +-\(vfree\|kvfree\) ++kvfree_sensitive + (E ++ , size + ) +); … > +coccilib.org.print_todo(p[0], > + "WARNING: opportunity for kzfree/kvfree_sensitive") I suggest to align the second function parameter. +coccilib.org.print_todo(p[0], +"WARNING: opportunity for kzfree/kvfree_sensitive") Regards, Markus ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH] coccinelle: misc: add array_size_dup script to detect missed overflow checks
I suggest to avoid a typo in the previous patch subject. … > +virtual context > +virtual report > +virtual org +virtual context, report, org Is such a SmPL code variant more succinct? … > +@as_next@ > +expression subE1 <= as.E1; > +expression as.E1; … I propose to reduce the repetition of this SmPL key word. … > + ... when != \(E1\|E2\|subE1\|subE2\)=E3 > + when != \(E1\|E2\|subE1\|subE2\)+=E3 … Can it make sense to express a constraint for a metavariable of the type “assignment operator”? > + when != \(\|\|\|\) How do you think about to use the following code exclusion specification? + when != & \(E1 \| E2 \| subE1 \| subE2\) … > +msg = "WARNING: same struct_size (line %s)" % (p1[0].line) > +coccilib.org.print_todo(p2[0], msg) I suggest once more to pass the desired message object directly as a function argument (without using an extra Python variable). Regards, Markus ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] coccinelle issues
> > Note2: https://github.com/coccinelle/coccinelle/blob/master/install.txt > > says that 'spatch' is a script, but it seems to be a binary executable file. > > Actually, it is a script, and the fact that you say it is a binary may be > the reason for your python problem. Normally there is a script > (scripts/spatch) that make install puts in place that refers back to where > your Coccinelle is installed. I suggest to take another look at the corresponding software development history. The build infrastructures were occasionally updated in the meantime. elfring@Sonne:~> SP=$(which spatch) && file $SP && du -h $SP /usr/local/bin/spatch: ELF 64-bit LSB executable, x86-64, version 1 (SYSV), dynamically linked, interpreter /lib64/ld-linux-x86-64.so.2, for GNU/Linux 3.2.0, with debug_info, not stripped 16M /usr/local/bin/spatch Regards, Markus ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] coccinelle: api: add kvfree script
> … Looks like it's impossible to break "when" lines. Thus I became also curious on clarification for further software development possibilities around the safer application of code exclusion specifications by the means of the semantic patch language. Regards, Markus ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH 1/3] parsing_cocci: arity: Apply mcode2arity on macrodecl/param attrs
> The following commits introduced some poor management of attributes in arity.ml: > 08f1cd9fb83ec400435e0ad8fdf579ec8f9c0f21 > b4b8653bd5a9607922e0050fe2fede10d422b218 I suggest to specify also subjects for such commit references. > Apply mcode2arity to macrodecl and parameter attributes. To which item does the identifier “mcode2arity” refer? Regards, Markus ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Checking software behaviour according to selected SmPL code variants
>> @display@ >> expression e; >> @@ >> *brelse(e); >> <+... when != e = ... >> (e = ... >> | >> *e >> )...+> >> >> >> I would find it nicer if I do not need to repeat a code exclusion >> specification >> as the first element of a SmPL disjunction for this special use case. > > You don't. The when you have is useless. I am curious under which circumstances we will achieve a better common understanding for the shown application of SmPL disjunctions eventually together with additional code exclusion specifications. Regards, Markus ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Checking software behaviour according to selected SmPL code variants
>> I hoped that my specification of a SmPL code exclusion should prevent >> the presentation of assignments (independent from statements and/or >> expressions). > > ( > e = e1 > | > *e > ) @display@ expression e; @@ *brelse(e); <+... when != e = ... (e = ... | *e )...+> I would find it nicer if I do not need to repeat a code exclusion specification as the first element of a SmPL disjunction for this special use case. Regards, Markus ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Checking software behaviour according to selected SmPL code variants
>> @display@ >> expression e; >> @@ >> *brelse(e); >> <+... when != e = ... >> *e >> ...+> … >> elfring@Sonne:~/Projekte/Linux/next-patched> spatch >> ~/Projekte/Coccinelle/janitor/show_questionable_brelse_usage8.cocci >> fs/ext4/extents.c >> … >> @@ -1127,8 +1121,6 @@ static int ext4_ext_split(handle_t *hand … >> -brelse(bh); >> -bh = NULL; … >> Would you like to suggest any fine-tuning for the search approach? > > When describes what happens elsewhere than in the statement matched by the > pattern. I hoped that my specification of a SmPL code exclusion should prevent the presentation of assignments (independent from statements and/or expressions). Regards, Markus ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Checking software behaviour according to selected SmPL code variants
> That is quite normal. One statement should be followed by another statement.
I have tried another script variant out for the semantic patch language.
@display@
expression e;
@@
*brelse(e);
<+... when != e = ...
*e
...+>
I wonder about the generation of a diff hunk then like the following.
elfring@Sonne:~/Projekte/Linux/next-patched> spatch
~/Projekte/Coccinelle/janitor/show_questionable_brelse_usage8.cocci
fs/ext4/extents.c
…
@@ -1127,8 +1121,6 @@ static int ext4_ext_split(handle_t *hand
err = ext4_handle_dirty_metadata(handle, inode, bh);
if (err)
goto cleanup;
- brelse(bh);
- bh = NULL;
/* correct old leaf */
if (m) {
…
Would you like to suggest any fine-tuning for the search approach?
Regards,
Markus
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Checking software behaviour according to selected SmPL code variants
>> If I omit the specification “, ...” from the function call parameters >> because I could be unsure about the number of arguments in other >> software situations, I do not get the desired test output as before. > > This has been discussed before. I was just looking for a related update in this area. > When you put <+... ...+> in an argument list, it doesn't mean an unknown > number > of arguments, it means one arguemnt that has something as a subexpression. Another SmPL code variant can eventually become interesting then. *y = x(..., <+... e ...+>, ...) >> If I omit even the semicolon from the assignment statement in the >> search pattern, I get an error message. >> >> elfring@Sonne:~/Projekte/Coccinelle/janitor> spatch --parse-cocci >> show_questionable_brelse_usage6.cocci >> … >> minus: parse error: >> File "show_questionable_brelse_usage6.cocci", line 6, column 0, charpos = >> 67 >> around = '', >> whole content = > > That is quite normal. One statement should be followed by another statement. I hope that the support will become better also for assignment expressions. Regards, Markus ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] Checking software behaviour according to selected SmPL code variants
Hello,
My software development attention was caught also by a recent patch.
https://lore.kernel.org/linux-fsdevel/20200608141629.GA1912173@mwanda/
https://lore.kernel.org/patchwork/patch/1253499/
Thus I have tried another tiny script out for the semantic patch language
(according to the software combination “Coccinelle 1.0.8-00104-ge06b9156”).
@display@
expression e, x, y;
@@
*brelse(e);
*y = x(<+... e ...+>, ...);
An usable output is generated then as expected for a test source file
like the following.
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/fs/exfat/nls.c?id=b676fdbcf4c8424f3c02ed7f31576d99b963bded#n652
// SPDX-License-Identifier: GPL-2.0-or-later
// deleted part
static int exfat_load_upcase_table(struct super_block *sb,
sector_t sector, unsigned long long num_sectors,
unsigned int utbl_checksum)
{
struct exfat_sb_info *sbi = EXFAT_SB(sb);
unsigned int sect_size = sb->s_blocksize;
unsigned int i, index = 0;
u32 chksum = 0;
// deleted part
while (sector < num_sectors) {
struct buffer_head *bh;
bh = sb_bread(sb, sector);
// deleted part
brelse(bh);
chksum = exfat_calc_chksum32(bh->b_data, i, chksum, CS_DEFAULT);
}
// deleted part
}
// deleted part
If I omit the specification “, ...” from the function call parameters
because I could be unsure about the number of arguments in other
software situations, I do not get the desired test output as before.
If I omit even the semicolon from the assignment statement in the
search pattern, I get an error message.
elfring@Sonne:~/Projekte/Coccinelle/janitor> spatch --parse-cocci
show_questionable_brelse_usage6.cocci
…
minus: parse error:
File "show_questionable_brelse_usage6.cocci", line 6, column 0, charpos = 67
around = '',
whole content =
Will such observations influence subsequent software evolution?
Regards,
Markus
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH v2 1/4] coccinelle: api: extend memdup_user transformation with GFP_USER
… > +++ b/scripts/coccinelle/api/memdup_user.cocci > @@ -20,7 +20,9 @@ expression from,to,size; … > +- to = \(kmalloc\|kzalloc\) > + (size,\(GFP_KERNEL\|GFP_USER\| > + \(GFP_KERNEL\|GFP_USER\)|__GFP_NOWARN\)); I got the impression that this SmPL code needs another correction also according to the proposed SmPL disjunction. +-to = \( kmalloc \| kzalloc \) (size, \( GFP_KERNEL \| GFP_USER \) \( | __GFP_NOWARN \| \) ); Would you like to express by any other approach that a specific flag is an optional source code transformation parameter? Regards, Markus ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH v2 4/4] coccinelle: api: add selfcheck for memdup_user rule
…
> +++ b/scripts/coccinelle/api/memdup_user.cocci
> @@ -14,13 +14,24 @@ virtual patch
> virtual context
> virtual org
> virtual report
> +virtual selfcheck
Would you like to avoid the repetition of a SmPL key word here?
+virtual patch, context, org, report, selfcheck
> @@ -117,3 +128,34 @@ p << rv.p;
> @@
>
> coccilib.report.print_report(p[0], "WARNING opportunity for vmemdup_user")
> +
> +@script:python depends on selfcheck@
> +@@
> +coccinelle.checked_files |= set(definitions.values()) & set(cocci.files())
I suggest to reconsider the usage of the function “cocci.files()”.
Can such a script rule determine for which file it should perform data
processing?
> +print('SELF-CHECK: the pattern no longer matches ' \
> + 'definitions {} in file {}'.format(not_found, efile))
Can the following code variant be a bit nicer?
+sys.stdout.write('SELF-CHECK: The pattern does not match
definitions {} in file {} any more.\n' \
+ .format(not_found, efile))
Regards,
Markus
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
