Re: [Cocci] transform oddity / bug ?
On Sun, 30 Aug 2020, Joe Perches wrote: > On Sun, 2020-08-30 at 17:46 +0200, Julia Lawall wrote: > > The following: > > > > @not_int_not_len exists@ > > type T != int; > > identifier x != len; > > position p; > > identifier sysfs =~ "^sysfs_emit.*$"; > > assignment operator aop; > > @@ > > > > T@p x; > > ... > > x aop sysfs(...) > > > > @@ > > type not_int_not_len.T; > > identifier not_int_not_len.x; > > position not_int_not_len.p; > > @@ > > > > ( > > - T@p x; > > + int len; > > - T@p x > > + int len > > = ...; > > ) > > <... > > - x > > + len > > ...> > > > > works on the following test file: > > > > int fn1() > > { > > ssize_t count = 0; > > count += sysfs_emit_at(buf, count, ...); > > return count; > > } > > > > int fn2() > > { > > ssize_t count; > > count += sysfs_emit_at(buf, count, ...); > > return count; > > } > > > > In the first rule, T@p x; benefits from an isomorphism to get the > > initialization case. That is not possible in the second rule, because the > > name of the declared variable is modified. > > Unfortunately this does not work when the declaration > is comma terminated and not semicolon terminated. I will have to look into it. It should handle this sort of thing, but it is somewhat complex, because the declarations have to be split and this specific case may not be handled. One thing that is possible is to change only the variable name. If there are not many occurrences, one could fix them up afterwards by hand. julia > > > $ cat julia.c > int fn1() > { > ssize_t count = 0, another; // <- multiple declarations > count += sysfs_emit_at(buf, count, ...); > return count; > } > > int fn2() > { > ssize_t count; > count += sysfs_emit_at(buf, count, ...); > return count; > } > > $ spatch -sp-file julia.cocci julia.c > init_defs_builtins: /usr/local/bin/../lib/coccinelle/standard.h > HANDLING: julia.c > diff = > --- julia.c > +++ /tmp/cocci-output-77064-888900-julia.c > @@ -7,8 +7,8 @@ int fn1() > > int fn2() > { > -ssize_t count; > -count += sysfs_emit_at(buf, count, ...); > -return count; > +int len; > +len += sysfs_emit_at(buf, count, ...); > +return len; > } > > > ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] transform oddity / bug ?
On Sun, 2020-08-30 at 17:46 +0200, Julia Lawall wrote: > The following: > > @not_int_not_len exists@ > type T != int; > identifier x != len; > position p; > identifier sysfs =~ "^sysfs_emit.*$"; > assignment operator aop; > @@ > > T@p x; > ... > x aop sysfs(...) > > @@ > type not_int_not_len.T; > identifier not_int_not_len.x; > position not_int_not_len.p; > @@ > > ( > - T@p x; > + int len; > - T@p x > + int len > = ...; > ) > <... > - x > + len > ...> > > works on the following test file: > > int fn1() > { > ssize_t count = 0; > count += sysfs_emit_at(buf, count, ...); > return count; > } > > int fn2() > { > ssize_t count; > count += sysfs_emit_at(buf, count, ...); > return count; > } > > In the first rule, T@p x; benefits from an isomorphism to get the > initialization case. That is not possible in the second rule, because the > name of the declared variable is modified. Unfortunately this does not work when the declaration is comma terminated and not semicolon terminated. $ cat julia.c int fn1() { ssize_t count = 0, another; // <- multiple declarations count += sysfs_emit_at(buf, count, ...); return count; } int fn2() { ssize_t count; count += sysfs_emit_at(buf, count, ...); return count; } $ spatch -sp-file julia.cocci julia.c init_defs_builtins: /usr/local/bin/../lib/coccinelle/standard.h HANDLING: julia.c diff = --- julia.c +++ /tmp/cocci-output-77064-888900-julia.c @@ -7,8 +7,8 @@ int fn1() int fn2() { -ssize_t count; -count += sysfs_emit_at(buf, count, ...); -return count; +int len; +len += sysfs_emit_at(buf, count, ...); +return len; } ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] transform oddity / bug ?
On Sun, 2020-08-30 at 17:46 +0200, Julia Lawall wrote: > [...] > > > Thanks, I used the slightly different from your suggestion > > where sysfs is an identifier with function args and a > > semicolon after the transform type, (otherwise I get cocci errors). > > like this below: > > > > But it doesn't work (no renaming) when there is an > > initializer to the variable to be transformed. > > > > ie: > > { > > ssize_t count = 0; > > ... > > count += sysfs_emit_at(buf, count, ...); > > ... > > return count; > > } > > > > I tried adding =0 in various places without success. > > > > Suggestions? > > > > -- > > > > // Rename the sysfs_emit assigned variables not named len and not already > > int > > // and set the name to len and type to int > > > > @not_int_not_len exists@ > > type T != int; > > identifier x != len; > > position p; > > identifier sysfs =~ "^sysfs_emit.*$"; > > assignment operator aop; > > @@ > > > > T x@p; > > ... > > x aop sysfs(...) > > ... > > > > @@ > > type not_int_not_len.T; > > identifier not_int_not_len.x; > > position not_int_not_len.p; > > @@ > > > > - T x@p; > > + int len; > > <... > > - x > > + len > > ...> > > > > -- > > The following: > > @not_int_not_len exists@ > type T != int; > identifier x != len; > position p; > identifier sysfs =~ "^sysfs_emit.*$"; > assignment operator aop; > @@ > > T@p x; > ... > x aop sysfs(...) > > @@ > type not_int_not_len.T; > identifier not_int_not_len.x; > position not_int_not_len.p; > @@ > > ( > - T@p x; > + int len; > - T@p x > + int len > = ...; > ) > <... > - x > + len > ...> > > works on the following test file: > > int fn1() > { > ssize_t count = 0; > count += sysfs_emit_at(buf, count, ...); > return count; > } > > int fn2() > { > ssize_t count; > count += sysfs_emit_at(buf, count, ...); > return count; > } > > In the first rule, T@p x; benefits from an isomorphism to get the > initialization case. That is not possible in the second rule, because the > name of the declared variable is modified. > > I wonder why you use a regular expression for the sysfs identifier. I > thought that there were only two choices? You will get better performance > if you make those two choices explicit in the rule, with \( \| \). Just because I'm accustomed to regex. I'll change it, Thanks for all the comments and corrections. cheers, Joe ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] transform oddity / bug ?
[...] > Thanks, I used the slightly different from your suggestion > where sysfs is an identifier with function args and a > semicolon after the transform type, (otherwise I get cocci errors). > like this below: > > But it doesn't work (no renaming) when there is an > initializer to the variable to be transformed. > > ie: > { > ssize_t count = 0; > ... > count += sysfs_emit_at(buf, count, ...); > ... > return count; > } > > I tried adding =0 in various places without success. > > Suggestions? > > -- > > // Rename the sysfs_emit assigned variables not named len and not already int > // and set the name to len and type to int > > @not_int_not_len exists@ > type T != int; > identifier x != len; > position p; > identifier sysfs =~ "^sysfs_emit.*$"; > assignment operator aop; > @@ > > T x@p; > ... > x aop sysfs(...) > ... > > @@ > type not_int_not_len.T; > identifier not_int_not_len.x; > position not_int_not_len.p; > @@ > > - T x@p; > + int len; > <... > - x > + len > ...> > > -- The following: @not_int_not_len exists@ type T != int; identifier x != len; position p; identifier sysfs =~ "^sysfs_emit.*$"; assignment operator aop; @@ T@p x; ... x aop sysfs(...) @@ type not_int_not_len.T; identifier not_int_not_len.x; position not_int_not_len.p; @@ ( - T@p x; + int len; | - T@p x + int len = ...; ) <... - x + len ...> works on the following test file: int fn1() { ssize_t count = 0; count += sysfs_emit_at(buf, count, ...); return count; } int fn2() { ssize_t count; count += sysfs_emit_at(buf, count, ...); return count; } In the first rule, T@p x; benefits from an isomorphism to get the initialization case. That is not possible in the second rule, because the name of the declared variable is modified. I wonder why you use a regular expression for the sysfs identifier. I thought that there were only two choices? You will get better performance if you make those two choices explicit in the rule, with \( \| \). julia ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] transform oddity / bug ?
On Sun, 2020-08-30 at 10:27 +0200, Julia Lawall wrote: > > On Sun, 30 Aug 2020, Joe Perches wrote: > > > On Sun, 2020-08-30 at 08:57 +0200, Julia Lawall wrote: > > > On Sat, 29 Aug 2020, Joe Perches wrote: > > > > > > > Is it me not understanding cocci grammar again? > > > > > > The problem is the loop. You are trying to change something in the body > > > of a loop and the body of a for loop might not be executed. ... means > > > that the thing must be found on every execution path. > > > > > > Do you want to make the change in the function header even if there are > > > not changes in the body? If so, <... ...> is what you are looking for. > > > If you want to be sure there is a change to make in the function body then > > > you can use <+... ...+> but it will be more expensive. > > > > Thanks. <... works (and I thought I had tried that, oh well) > > > > Another thing I'd like to do is to change the various uses > > of a type and identifier to a specific type and identifier. > > > > In this sysfs_emit transform I've been working on, there > > are many variable names used in the assignment of the > > sysfs_emit result. > > > > $ git grep -P -oh '\w+\s*\+?=\s*sysfs_emit' | \ > > sort | uniq -c | sort -rn > > 145 ret = sysfs_emit > > 80 len = sysfs_emit > > 74 len += sysfs_emit > > 69 rc = sysfs_emit > > 50 count = sysfs_emit > > 25 count += sysfs_emit > > 19 size = sysfs_emit > > 17 n += sysfs_emit > > 15 n = sysfs_emit > > 14 status = sysfs_emit > > 12 ret += sysfs_emit > > 12 output_len += sysfs_emit > > 11 retval = sysfs_emit > > 9 res += sysfs_emit > > 7 rv = sysfs_emit > > 7 offset += sysfs_emit > > 7 l = sysfs_emit > > 6 i = sysfs_emit > > 5 size += sysfs_emit > > 5 err = sysfs_emit > > 4 written = sysfs_emit > > 4 l += sysfs_emit > > 3 written += sysfs_emit > > 2 rz = sysfs_emit > > 2 r = sysfs_emit > > 2 result = sysfs_emit > > 2 res = sysfs_emit > > 2 i += sysfs_emit > > 2 idx += sysfs_emit > > 2 error = sysfs_emit > > 2 cnt += sysfs_emit > > 2 buf_len += sysfs_emit > > 1 offset = sysfs_emit > > 1 length = sysfs_emit > > 1 cnt = sysfs_emit > > 1 bytes = sysfs_emit > > 1 bytes += sysfs_emit > > > > Most are declared int, some are ssize_t. > > > > I'd like to change them all to int len. > > @r exists@ > type T != int; > identifier x != len; > position p; > assignment operator aop; > @@ > > T x@p; > ... > x aop sysfs_emit > > @@ > type r.T; > identifier r.x; > position r.p; > @@ > > - T x@p; > + int len > <... > - x > + len > ...> > > This only works for the case where the type is not int and the name is not > len. You would need other similar pairs of rules for the case where the > type is int and the variable is something else and where the type is len > and the variable is len. Thanks, I used the slightly different from your suggestion where sysfs is an identifier with function args and a semicolon after the transform type, (otherwise I get cocci errors). like this below: But it doesn't work (no renaming) when there is an initializer to the variable to be transformed. ie: { ssize_t count = 0; ... count += sysfs_emit_at(buf, count, ...); ... return count; } I tried adding =0 in various places without success. Suggestions? -- // Rename the sysfs_emit assigned variables not named len and not already int // and set the name to len and type to int @not_int_not_len exists@ type T != int; identifier x != len; position p; identifier sysfs =~ "^sysfs_emit.*$"; assignment operator aop; @@ T x@p; ... x aop sysfs(...) ... @@ type not_int_not_len.T; identifier not_int_not_len.x; position not_int_not_len.p; @@ - T x@p; + int len; <... - x + len ...> -- ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] transform oddity
On Sun, 30 Aug 2020, Markus Elfring wrote: > >> How do you think about to use the following transformation variant? > > > > This is uselessly expensive. It is already known that there is at least > > one occurrence of x. > > Would we like to care if the affected identifier should be updated more than > once > in a selected function implementation? Theer is no way to specify "more than once". We know it will be updated at least once because we know that there is at least one occurrence. julia ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] transform oddity
On Sun, 30 Aug 2020, Markus Elfring wrote: > > - T x@p; > > + int len > > <... > > - x > > + len > > ...> > > How do you think about to use the following transformation variant? This is uselessly expensive. It is already known that there is at least one occurrence of x. julia > > -T x@p; > +int len; > <+... > -x > +len > ...+> > > > Regards, > Markus > ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] transform oddity / bug ?
On Sun, 30 Aug 2020, Joe Perches wrote: > On Sun, 2020-08-30 at 08:57 +0200, Julia Lawall wrote: > > > > On Sat, 29 Aug 2020, Joe Perches wrote: > > > > > Is it me not understanding cocci grammar again? > > > > The problem is the loop. You are trying to change something in the body > > of a loop and the body of a for loop might not be executed. ... means > > that the thing must be found on every execution path. > > > > Do you want to make the change in the function header even if there are > > not changes in the body? If so, <... ...> is what you are looking for. > > If you want to be sure there is a change to make in the function body then > > you can use <+... ...+> but it will be more expensive. > > Thanks. <... works (and I thought I had tried that, oh well) > > Another thing I'd like to do is to change the various uses > of a type and identifier to a specific type and identifier. > > In this sysfs_emit transform I've been working on, there > are many variable names used in the assignment of the > sysfs_emit result. > > $ git grep -P -oh '\w+\s*\+?=\s*sysfs_emit' | \ > sort | uniq -c | sort -rn > 145 ret = sysfs_emit > 80 len = sysfs_emit > 74 len += sysfs_emit > 69 rc = sysfs_emit > 50 count = sysfs_emit > 25 count += sysfs_emit > 19 size = sysfs_emit > 17 n += sysfs_emit > 15 n = sysfs_emit > 14 status = sysfs_emit > 12 ret += sysfs_emit > 12 output_len += sysfs_emit > 11 retval = sysfs_emit > 9 res += sysfs_emit > 7 rv = sysfs_emit > 7 offset += sysfs_emit > 7 l = sysfs_emit > 6 i = sysfs_emit > 5 size += sysfs_emit > 5 err = sysfs_emit > 4 written = sysfs_emit > 4 l += sysfs_emit > 3 written += sysfs_emit > 2 rz = sysfs_emit > 2 r = sysfs_emit > 2 result = sysfs_emit > 2 res = sysfs_emit > 2 i += sysfs_emit > 2 idx += sysfs_emit > 2 error = sysfs_emit > 2 cnt += sysfs_emit > 2 buf_len += sysfs_emit > 1 offset = sysfs_emit > 1 length = sysfs_emit > 1 cnt = sysfs_emit > 1 bytes = sysfs_emit > 1 bytes += sysfs_emit > > Most are declared int, some are ssize_t. > > I'd like to change them all to int len. @r exists@ type T != int; identifier x != len; position p; assignment operator aop; @@ T x@p; ... x aop sysfs_emit @@ type r.T; identifier r.x; position r.p; @@ - T x@p; + int len <... - x + len ...> This only works for the case where the type is not int and the name is not len. You would need other similar pairs of rules for the case where the type is int and the variable is something else and where the type is len and the variable is len. Or you could get rid of the constraints and hope that replacing int by int and len by len won't have any impact on the layout of the code. julia ___ 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] transform oddity / bug ?
On Sun, 2020-08-30 at 08:57 +0200, Julia Lawall wrote: > > On Sat, 29 Aug 2020, Joe Perches wrote: > > > Is it me not understanding cocci grammar again? > > The problem is the loop. You are trying to change something in the body > of a loop and the body of a for loop might not be executed. ... means > that the thing must be found on every execution path. > > Do you want to make the change in the function header even if there are > not changes in the body? If so, <... ...> is what you are looking for. > If you want to be sure there is a change to make in the function body then > you can use <+... ...+> but it will be more expensive. Thanks. <... works (and I thought I had tried that, oh well) Another thing I'd like to do is to change the various uses of a type and identifier to a specific type and identifier. In this sysfs_emit transform I've been working on, there are many variable names used in the assignment of the sysfs_emit result. $ git grep -P -oh '\w+\s*\+?=\s*sysfs_emit' | \ sort | uniq -c | sort -rn 145 ret = sysfs_emit 80 len = sysfs_emit 74 len += sysfs_emit 69 rc = sysfs_emit 50 count = sysfs_emit 25 count += sysfs_emit 19 size = sysfs_emit 17 n += sysfs_emit 15 n = sysfs_emit 14 status = sysfs_emit 12 ret += sysfs_emit 12 output_len += sysfs_emit 11 retval = sysfs_emit 9 res += sysfs_emit 7 rv = sysfs_emit 7 offset += sysfs_emit 7 l = sysfs_emit 6 i = sysfs_emit 5 size += sysfs_emit 5 err = sysfs_emit 4 written = sysfs_emit 4 l += sysfs_emit 3 written += sysfs_emit 2 rz = sysfs_emit 2 r = sysfs_emit 2 result = sysfs_emit 2 res = sysfs_emit 2 i += sysfs_emit 2 idx += sysfs_emit 2 error = sysfs_emit 2 cnt += sysfs_emit 2 buf_len += sysfs_emit 1 offset = sysfs_emit 1 length = sysfs_emit 1 cnt = sysfs_emit 1 bytes = sysfs_emit 1 bytes += sysfs_emit Most are declared int, some are ssize_t. I'd like to change them all to int len. Any suggestions? ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci