[Cocci] transform oddity / bug ?

[Cocci] transform oddity / bug ?

[Joe Perches]

[-- Attachment #1: Type: text/plain, Size: 2113 bytes --]

Is it me not understanding cocci grammar again?

Given these input and cocci script files:

Why isn't the show_test1 function transformed?
Why is only the show_test2 function transformed?

The only difference between the files is some
commented out lines with a for loop and if test.

$ cat test.c
static ssize_t test1_show(struct device *d,
		struct device_attribute *a,
		char *buffer)
{
	ssize_t rc = 0;

	for (cnt = 0; cnt < s_attr->size; cnt++) {
		if (cnt && !(cnt % 16)) {
			if (PAGE_SIZE - rc)
				buffer[rc++] = '\n';
		}

		rc += scnprintf(buffer + rc, PAGE_SIZE - rc, "%02x ",
				((unsigned char *)s_attr->data)[cnt]);
	}
	return rc;
}

static ssize_t test2_show(struct device *d,
		struct device_attribute *a,
		char *buffer)
{
	ssize_t rc = 0;

//	for (cnt = 0; cnt < s_attr->size; cnt++) {
//		if (cnt && !(cnt % 16)) {
			if (PAGE_SIZE - rc)
				buffer[rc++] = '\n';
//		}
		rc += scnprintf(buffer + rc, PAGE_SIZE - rc, "%02x ",
				((unsigned char *)s_attr->data)[cnt]);
//	}
	return rc;
}
$ cat sysfs_emit_rename.cocci
@@
identifier d_show =~ "^.*show.*$";
identifier arg1, arg2, arg3;
@@
ssize_t d_show(struct device *
-	arg1
+	dev
	, struct device_attribute *
-	arg2
+	attr
	, char *
-	arg3
+	buf
	)
{
	...
(
-	arg1
+	dev
|
-	arg2
+	attr
|
-	arg3
+	buf
)
	... when any
}
$ spatch -sp-file sysfs_emit_rename.cocci test.c 
init_defs_builtins: /usr/local/bin/../lib/coccinelle/standard.h
HANDLING: test.c
diff = 
--- test.c
+++ /tmp/cocci-output-68270-4c9b1f-test.c
@@ -16,18 +16,18 @@ static ssize_t test1_show(struct device
 	return rc;
 }
 
-static ssize_t test2_show(struct device *d,
-		struct device_attribute *a,
-		char *buffer)
+static ssize_t test2_show(struct device *dev,
+			  struct device_attribute *attr,
+			  char *buf)
 {
 	ssize_t rc = 0;
 
 //	for (cnt = 0; cnt < s_attr->size; cnt++) {
 //		if (cnt && !(cnt % 16)) {
 			if (PAGE_SIZE - rc)
-				buffer[rc++] = '\n';
+				buf[rc++] = '\n';
 //		}
-		rc += scnprintf(buffer + rc, PAGE_SIZE - rc, "%02x ",
+		rc += scnprintf(buf + rc, PAGE_SIZE - rc, "%02x ",
 				((unsigned char *)s_attr->data)[cnt]);
 //	}
 	return rc;
$

[-- Attachment #2: test.c --]
[-- Type: text/x-csrc, Size: 711 bytes --]

static ssize_t test1_show(struct device *d,
		struct device_attribute *a,
		char *buffer)
{
	ssize_t rc = 0;

	for (cnt = 0; cnt < s_attr->size; cnt++) {
		if (cnt && !(cnt % 16)) {
			if (PAGE_SIZE - rc)
				buffer[rc++] = '\n';
		}

		rc += scnprintf(buffer + rc, PAGE_SIZE - rc, "%02x ",
				((unsigned char *)s_attr->data)[cnt]);
	}
	return rc;
}

static ssize_t test2_show(struct device *d,
		struct device_attribute *a,
		char *buffer)
{
	ssize_t rc = 0;

//	for (cnt = 0; cnt < s_attr->size; cnt++) {
//		if (cnt && !(cnt % 16)) {
			if (PAGE_SIZE - rc)
				buffer[rc++] = '\n';
//		}
		rc += scnprintf(buffer + rc, PAGE_SIZE - rc, "%02x ",
				((unsigned char *)s_attr->data)[cnt]);
//	}
	return rc;
}


[-- Attachment #3: sysfs_emit_rename.cocci --]
[-- Type: text/plain, Size: 254 bytes --]

@@
identifier d_show =~ "^.*show.*$";
identifier arg1, arg2, arg3;
@@
ssize_t d_show(struct device *
-	arg1
+	dev
	, struct device_attribute *
-	arg2
+	attr
	, char *
-	arg3
+	buf
	)
{
	...
(
-	arg1
+	dev
|
-	arg2
+	attr
|
-	arg3
+	buf
)
	... when any
}

[-- Attachment #4: Type: text/plain, Size: 136 bytes --]

_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] transform oddity / bug ?

[Julia Lawall]

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.

julia

>
> Given these input and cocci script files:
>
> Why isn't the show_test1 function transformed?
> Why is only the show_test2 function transformed?
>
> The only difference between the files is some
> commented out lines with a for loop and if test.
>
> $ cat test.c
> static ssize_t test1_show(struct device *d,
> 		struct device_attribute *a,
> 		char *buffer)
> {
> 	ssize_t rc = 0;
>
> 	for (cnt = 0; cnt < s_attr->size; cnt++) {
> 		if (cnt && !(cnt % 16)) {
> 			if (PAGE_SIZE - rc)
> 				buffer[rc++] = '\n';
> 		}
>
> 		rc += scnprintf(buffer + rc, PAGE_SIZE - rc, "%02x ",
> 				((unsigned char *)s_attr->data)[cnt]);
> 	}
> 	return rc;
> }
>
> static ssize_t test2_show(struct device *d,
> 		struct device_attribute *a,
> 		char *buffer)
> {
> 	ssize_t rc = 0;
>
> //	for (cnt = 0; cnt < s_attr->size; cnt++) {
> //		if (cnt && !(cnt % 16)) {
> 			if (PAGE_SIZE - rc)
> 				buffer[rc++] = '\n';
> //		}
> 		rc += scnprintf(buffer + rc, PAGE_SIZE - rc, "%02x ",
> 				((unsigned char *)s_attr->data)[cnt]);
> //	}
> 	return rc;
> }
> $ cat sysfs_emit_rename.cocci
> @@
> identifier d_show =~ "^.*show.*$";
> identifier arg1, arg2, arg3;
> @@
> ssize_t d_show(struct device *
> -	arg1
> +	dev
> 	, struct device_attribute *
> -	arg2
> +	attr
> 	, char *
> -	arg3
> +	buf
> 	)
> {
> 	...
> (
> -	arg1
> +	dev
> |
> -	arg2
> +	attr
> |
> -	arg3
> +	buf
> )
> 	... when any
> }
> $ spatch -sp-file sysfs_emit_rename.cocci test.c
> init_defs_builtins: /usr/local/bin/../lib/coccinelle/standard.h
> HANDLING: test.c
> diff =
> --- test.c
> +++ /tmp/cocci-output-68270-4c9b1f-test.c
> @@ -16,18 +16,18 @@ static ssize_t test1_show(struct device
>  	return rc;
>  }
>
> -static ssize_t test2_show(struct device *d,
> -		struct device_attribute *a,
> -		char *buffer)
> +static ssize_t test2_show(struct device *dev,
> +			  struct device_attribute *attr,
> +			  char *buf)
>  {
>  	ssize_t rc = 0;
>
>  //	for (cnt = 0; cnt < s_attr->size; cnt++) {
>  //		if (cnt && !(cnt % 16)) {
>  			if (PAGE_SIZE - rc)
> -				buffer[rc++] = '\n';
> +				buf[rc++] = '\n';
>  //		}
> -		rc += scnprintf(buffer + rc, PAGE_SIZE - rc, "%02x ",
> +		rc += scnprintf(buf + rc, PAGE_SIZE - rc, "%02x ",
>  				((unsigned char *)s_attr->data)[cnt]);
>  //	}
>  	return rc;
> $
>
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] transform oddity / bug ?

[Joe Perches]

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

Re: [Cocci] transform oddity / bug ?

[Julia Lawall]

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 / bug ?

[Joe Perches]

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 / bug ?

[Julia Lawall]

[...]

> 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 ?

[Joe Perches]

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 ?

[Joe Perches]

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 ?

[Julia Lawall]

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 ?

[Joe Perches]

On Sun, 2020-08-30 at 20:41 +0200, Julia Lawall wrote:
> On Sun, 30 Aug 2020, Joe Perches wrote:
> > On Sun, 2020-08-30 at 17:46 +0200, Julia Lawall wrote:
> > > 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.

Thanks.  Hope you can get to look at that one day.

> 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.

And hi again Julia.

I've tried a few variations on finding uses of a function
argument that are not by specific named functions or with
local assignment of that function argument to another
variable without success.

For example:

ssize_t fn(struct device *dev, struct device_attribute *attr, char *buf)
{
*	char *orig = buf;
or
	int count;
...
*	count = local_static_func(some_struct *foo, buf);
}

where local_static_func is not sysfs_emit or sysfs_emit_at

Any clue you can offer?

_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] transform oddity / bug ?

[Julia Lawall]

On Wed, 2 Sep 2020, Joe Perches wrote:

> On Sun, 2020-08-30 at 20:41 +0200, Julia Lawall wrote:
> > On Sun, 30 Aug 2020, Joe Perches wrote:
> > > On Sun, 2020-08-30 at 17:46 +0200, Julia Lawall wrote:
> > > > 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.
>
> Thanks.  Hope you can get to look at that one day.
>
> > 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.
>
> And hi again Julia.
>
> I've tried a few variations on finding uses of a function
> argument that are not by specific named functions or with
> local assignment of that function argument to another
> variable without success.
>
> For example:
>
> ssize_t fn(struct device *dev, struct device_attribute *attr, char *buf)
> {
> *	char *orig = buf;
> or
> 	int count;
> ...
> *	count = local_static_func(some_struct *foo, buf);
> }
>
> where local_static_func is not sysfs_emit or sysfs_emit_at
>
> Any clue you can offer?

I'm not completely sure what is wanted.  Are you always concerned with the
buf parameter, or with any parameter.  It seems that there are uses that
are ok and uses that you want to point out.  What happens if there are
both in the same function?  Do you want to point something out only if
there are no good uses?  Or even if there are also good uses?

One option would be a sequence of rules that match things that you are ok
with.  Then a final match that makes a report for things that don't match
the previous rules.

Perhaps an option could be:

@protected@
identifier fn,buf;
position p1,p2;
@@

ssize_t fn@p1(...,char *buf) { <...
\(sysfs_emit\|sysfs_emit_at\)(...,buf@p2,...)
...> }

@@
identifier protected.fn,protected.buf;
position protected.p1;
position p2 != protected.p2;
@@

ssize_t fn@p1(...,char *buf) { <...
*buf@p2
...> }

In the first rule, if buf could be a subexpression of an argument of
sysfs_emit or sysfs_emit_at, then you can put <+... buf@p2 ...+>.

julia
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] transform oddity / bug ?

[Joe Perches]

On Wed, 2020-09-02 at 22:46 +0200, Julia Lawall wrote:
> 
> On Wed, 2 Sep 2020, Joe Perches wrote:
> 
> > On Sun, 2020-08-30 at 20:41 +0200, Julia Lawall wrote:
> > > On Sun, 30 Aug 2020, Joe Perches wrote:
> > > > On Sun, 2020-08-30 at 17:46 +0200, Julia Lawall wrote:
> > > > > 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.
> > 
> > Thanks.  Hope you can get to look at that one day.
> > 
> > > 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.
> > 
> > And hi again Julia.
> > 
> > I've tried a few variations on finding uses of a function
> > argument that are not by specific named functions or with
> > local assignment of that function argument to another
> > variable without success.
> > 
> > For example:
> > 
> > ssize_t fn(struct device *dev, struct device_attribute *attr, char *buf)
> > {
> > *	char *orig = buf;
> > or
> > 	int count;
> > ...
> > *	count = local_static_func(some_struct *foo, buf);
> > }
> > 
> > where local_static_func is not sysfs_emit or sysfs_emit_at
> > 
> > Any clue you can offer?
> 
> I'm not completely sure what is wanted.

I want to identify places where the 3rd argument
to any sysfs "show" function is used other than
by the converted sysfs_emit and sysfs_emit_to calls.

For instance, this assignment of p to buf

drivers/isdn/mISDN/dsp_pipeline.c-static ssize_t
drivers/isdn/mISDN/dsp_pipeline.c-attr_show_args(struct device *dev, struct device_attribute *attr, char *buf)
drivers/isdn/mISDN/dsp_pipeline.c-{
drivers/isdn/mISDN/dsp_pipeline.c-      struct mISDN_dsp_element *elem = dev_get_drvdata(dev);
drivers/isdn/mISDN/dsp_pipeline.c-      int i;
drivers/isdn/mISDN/dsp_pipeline.c-      char *p = buf;
drivers/isdn/mISDN/dsp_pipeline.c-
drivers/isdn/mISDN/dsp_pipeline.c:      *buf = 0;
drivers/isdn/mISDN/dsp_pipeline.c-      for (i = 0; i < elem->num_args; i++)
drivers/isdn/mISDN/dsp_pipeline.c-              p += sprintf(p, "Name:        %s\n%s%s%sDescription: %s\n\n",
drivers/isdn/mISDN/dsp_pipeline.c-                           elem->args[i].name,
drivers/isdn/mISDN/dsp_pipeline.c-                           elem->args[i].def ? "Default:     " : "",
drivers/isdn/mISDN/dsp_pipeline.c-                           elem->args[i].def ? elem->args[i].def : "",
drivers/isdn/mISDN/dsp_pipeline.c-                           elem->args[i].def ? "\n" : "",
drivers/isdn/mISDN/dsp_pipeline.c-                           elem->args[i].desc);
drivers/isdn/mISDN/dsp_pipeline.c-
drivers/isdn/mISDN/dsp_pipeline.c-      return p - buf;
drivers/isdn/mISDN/dsp_pipeline.c-}

so that this could (likely manually) be converted to:

static ssize_t attr_show_args(struct device *dev, struct device_attribute *attr, char *buf)
{
	struct mISDN_dsp_element *elem = dev_get_drvdata(dev);
	int len = 0;

	for (i = 0; i < elem->num_args; i++) {
		len += sysfs_emit(buf, len, "Name:        %s\n%s%s%sDescription: %s\n\n",
				  elem->args[i].name,
				  elem->args[i].def ? "Default:     " : "",
				  elem->args[i].def ? elem->args[i].def : "",
				  elem->args[i].def ? "\n" : "",
				  elem->args[i].desc);
	}

	return len;
}

And any use of buf passed to another function:

drivers/macintosh/macio_sysfs.c-static ssize_t modalias_show (struct device *dev,
drivers/macintosh/macio_sysfs.c-                              struct device_attribute *attr,
drivers/macintosh/macio_sysfs.c-                              char *buf)
drivers/macintosh/macio_sysfs.c-{
drivers/macintosh/macio_sysfs.c-        return of_device_modalias(dev, buf, PAGE_SIZE);
drivers/macintosh/macio_sysfs.c-}


_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] transform oddity / bug ?

[Julia Lawall]

On Wed, 2 Sep 2020, Joe Perches wrote:

> On Wed, 2020-09-02 at 22:46 +0200, Julia Lawall wrote:
> >
> > On Wed, 2 Sep 2020, Joe Perches wrote:
> >
> > > On Sun, 2020-08-30 at 20:41 +0200, Julia Lawall wrote:
> > > > On Sun, 30 Aug 2020, Joe Perches wrote:
> > > > > On Sun, 2020-08-30 at 17:46 +0200, Julia Lawall wrote:
> > > > > > 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.
> > >
> > > Thanks.  Hope you can get to look at that one day.
> > >
> > > > 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.
> > >
> > > And hi again Julia.
> > >
> > > I've tried a few variations on finding uses of a function
> > > argument that are not by specific named functions or with
> > > local assignment of that function argument to another
> > > variable without success.
> > >
> > > For example:
> > >
> > > ssize_t fn(struct device *dev, struct device_attribute *attr, char *buf)
> > > {
> > > *	char *orig = buf;
> > > or
> > > 	int count;
> > > ...
> > > *	count = local_static_func(some_struct *foo, buf);
> > > }
> > >
> > > where local_static_func is not sysfs_emit or sysfs_emit_at
> > >
> > > Any clue you can offer?
> >
> > I'm not completely sure what is wanted.
>
> I want to identify places where the 3rd argument
> to any sysfs "show" function is used other than
> by the converted sysfs_emit and sysfs_emit_to calls.
>
> For instance, this assignment of p to buf
>
> drivers/isdn/mISDN/dsp_pipeline.c-static ssize_t
> drivers/isdn/mISDN/dsp_pipeline.c-attr_show_args(struct device *dev, struct device_attribute *attr, char *buf)
> drivers/isdn/mISDN/dsp_pipeline.c-{
> drivers/isdn/mISDN/dsp_pipeline.c-      struct mISDN_dsp_element *elem = dev_get_drvdata(dev);
> drivers/isdn/mISDN/dsp_pipeline.c-      int i;
> drivers/isdn/mISDN/dsp_pipeline.c-      char *p = buf;
> drivers/isdn/mISDN/dsp_pipeline.c-
> drivers/isdn/mISDN/dsp_pipeline.c:      *buf = 0;
> drivers/isdn/mISDN/dsp_pipeline.c-      for (i = 0; i < elem->num_args; i++)
> drivers/isdn/mISDN/dsp_pipeline.c-              p += sprintf(p, "Name:        %s\n%s%s%sDescription: %s\n\n",
> drivers/isdn/mISDN/dsp_pipeline.c-                           elem->args[i].name,
> drivers/isdn/mISDN/dsp_pipeline.c-                           elem->args[i].def ? "Default:     " : "",
> drivers/isdn/mISDN/dsp_pipeline.c-                           elem->args[i].def ? elem->args[i].def : "",
> drivers/isdn/mISDN/dsp_pipeline.c-                           elem->args[i].def ? "\n" : "",
> drivers/isdn/mISDN/dsp_pipeline.c-                           elem->args[i].desc);
> drivers/isdn/mISDN/dsp_pipeline.c-
> drivers/isdn/mISDN/dsp_pipeline.c-      return p - buf;
> drivers/isdn/mISDN/dsp_pipeline.c-}
>
> so that this could (likely manually) be converted to:
>
> static ssize_t attr_show_args(struct device *dev, struct device_attribute *attr, char *buf)
> {
> 	struct mISDN_dsp_element *elem = dev_get_drvdata(dev);
> 	int len = 0;
>
> 	for (i = 0; i < elem->num_args; i++) {
> 		len += sysfs_emit(buf, len, "Name:        %s\n%s%s%sDescription: %s\n\n",
> 				  elem->args[i].name,
> 				  elem->args[i].def ? "Default:     " : "",
> 				  elem->args[i].def ? elem->args[i].def : "",
> 				  elem->args[i].def ? "\n" : "",
> 				  elem->args[i].desc);
> 	}
>
> 	return len;
> }
>
> And any use of buf passed to another function:
>
> drivers/macintosh/macio_sysfs.c-static ssize_t modalias_show (struct device *dev,
> drivers/macintosh/macio_sysfs.c-                              struct device_attribute *attr,
> drivers/macintosh/macio_sysfs.c-                              char *buf)
> drivers/macintosh/macio_sysfs.c-{
> drivers/macintosh/macio_sysfs.c-        return of_device_modalias(dev, buf, PAGE_SIZE);
> drivers/macintosh/macio_sysfs.c-}

OK, I think that what I suggested could work.

julia
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] transform oddity / bug ?

[Joe Perches]

On Wed, 2020-09-02 at 23:38 +0200, Julia Lawall wrote:
> On Wed, 2 Sep 2020, Joe Perches wrote:
> > On Wed, 2020-09-02 at 22:46 +0200, Julia Lawall wrote:
> > > On Wed, 2 Sep 2020, Joe Perches wrote:
> > > > On Sun, 2020-08-30 at 20:41 +0200, Julia Lawall wrote:
> > > > > On Sun, 30 Aug 2020, Joe Perches wrote:
> > > > > > On Sun, 2020-08-30 at 17:46 +0200, Julia Lawall wrote:
> > > > > > > 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.
> > > > 
> > > > Thanks.  Hope you can get to look at that one day.
> > > > 
> > > > > 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.
> > > > 
> > > > And hi again Julia.
> > > > 
> > > > I've tried a few variations on finding uses of a function
> > > > argument that are not by specific named functions or with
> > > > local assignment of that function argument to another
> > > > variable without success.
> > > > 
> > > > For example:
> > > > 
> > > > ssize_t fn(struct device *dev, struct device_attribute *attr, char *buf)
> > > > {
> > > > *	char *orig = buf;
> > > > or
> > > > 	int count;
> > > > ...
> > > > *	count = local_static_func(some_struct *foo, buf);
> > > > }
> > > > 
> > > > where local_static_func is not sysfs_emit or sysfs_emit_at
> > > > 
> > > > Any clue you can offer?
> > > 
> > > I'm not completely sure what is wanted.
> > 
> > I want to identify places where the 3rd argument
> > to any sysfs "show" function is used other than
> > by the converted sysfs_emit and sysfs_emit_to calls.
> > 
> > For instance, this assignment of p to buf
> > 
> > drivers/isdn/mISDN/dsp_pipeline.c-static ssize_t
> > drivers/isdn/mISDN/dsp_pipeline.c-attr_show_args(struct device *dev, struct device_attribute *attr, char *buf)
> > drivers/isdn/mISDN/dsp_pipeline.c-{
> > drivers/isdn/mISDN/dsp_pipeline.c-      struct mISDN_dsp_element *elem = dev_get_drvdata(dev);
> > drivers/isdn/mISDN/dsp_pipeline.c-      int i;
> > drivers/isdn/mISDN/dsp_pipeline.c-      char *p = buf;
> > drivers/isdn/mISDN/dsp_pipeline.c-
> > drivers/isdn/mISDN/dsp_pipeline.c:      *buf = 0;
> > drivers/isdn/mISDN/dsp_pipeline.c-      for (i = 0; i < elem->num_args; i++)
> > drivers/isdn/mISDN/dsp_pipeline.c-              p += sprintf(p, "Name:        %s\n%s%s%sDescription: %s\n\n",
> > drivers/isdn/mISDN/dsp_pipeline.c-                           elem->args[i].name,
> > drivers/isdn/mISDN/dsp_pipeline.c-                           elem->args[i].def ? "Default:     " : "",
> > drivers/isdn/mISDN/dsp_pipeline.c-                           elem->args[i].def ? elem->args[i].def : "",
> > drivers/isdn/mISDN/dsp_pipeline.c-                           elem->args[i].def ? "\n" : "",
> > drivers/isdn/mISDN/dsp_pipeline.c-                           elem->args[i].desc);
> > drivers/isdn/mISDN/dsp_pipeline.c-
> > drivers/isdn/mISDN/dsp_pipeline.c-      return p - buf;
> > drivers/isdn/mISDN/dsp_pipeline.c-}
> > 
> > so that this could (likely manually) be converted to:
> > 
> > static ssize_t attr_show_args(struct device *dev, struct device_attribute *attr, char *buf)
> > {
> > 	struct mISDN_dsp_element *elem = dev_get_drvdata(dev);
> > 	int len = 0;
> > 
> > 	for (i = 0; i < elem->num_args; i++) {
> > 		len += sysfs_emit(buf, len, "Name:        %s\n%s%s%sDescription: %s\n\n",
> > 				  elem->args[i].name,
> > 				  elem->args[i].def ? "Default:     " : "",
> > 				  elem->args[i].def ? elem->args[i].def : "",
> > 				  elem->args[i].def ? "\n" : "",
> > 				  elem->args[i].desc);
> > 	}
> > 
> > 	return len;
> > }
> > 
> > And any use of buf passed to another function:
> > 
> > drivers/macintosh/macio_sysfs.c-static ssize_t modalias_show (struct device *dev,
> > drivers/macintosh/macio_sysfs.c-                              struct device_attribute *attr,
> > drivers/macintosh/macio_sysfs.c-                              char *buf)
> > drivers/macintosh/macio_sysfs.c-{
> > drivers/macintosh/macio_sysfs.c-        return of_device_modalias(dev, buf, PAGE_SIZE);
> > drivers/macintosh/macio_sysfs.c-}
> 
> OK, I think that what I suggested could work.

Yes, it did point the way, thanks.

I needed to change the protected block to use a
sysfs_ function specific definition, otherwise
additional similar, but not sysfs <show> function
blocks were also output.

cheers, Joe

----

@dev_protected@
identifier fn,buf;
identifier dev, attr;
position p1,p2;
@@

ssize_t fn@p1(struct device *dev, struct device_attribute *attr, char *buf) {
<...
\(sysfs_emit\|sysfs_emit_at\)(...,buf@p2,...)
...>
}

@@
identifier dev_protected.fn,dev_protected.buf;
position dev_protected.p1;
position p2 != dev_protected.p2;
@@

ssize_t fn@p1(...,char *buf) { <...
*buf@p2
...> }
 


_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] transform oddity / bug ?

[Julia Lawall]

On Wed, 2 Sep 2020, Joe Perches wrote:

> On Wed, 2020-09-02 at 22:46 +0200, Julia Lawall wrote:
> >
> > On Wed, 2 Sep 2020, Joe Perches wrote:
> >
> > > On Sun, 2020-08-30 at 20:41 +0200, Julia Lawall wrote:
> > > > On Sun, 30 Aug 2020, Joe Perches wrote:
> > > > > On Sun, 2020-08-30 at 17:46 +0200, Julia Lawall wrote:
> > > > > > 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.
> > >
> > > Thanks.  Hope you can get to look at that one day.

It works if you replace the addition of the new declaration by ++.  It
seems that it is concerned that if there are multiple variables in the
original declaration then it may be necessary to do multiple additions and
so it doesn't do anything for a single +.  You can see this information
with the --debug option.

The newlines in the generated code are also not what one would hope for.
I will see if this can be improved.

julia
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] transform oddity / bug ?

[Joe Perches]

On Thu, 2020-09-03 at 17:14 +0200, Julia Lawall wrote:
> On Wed, 2 Sep 2020, Joe Perches wrote:
> > On Wed, 2020-09-02 at 22:46 +0200, Julia Lawall wrote:
> > > On Wed, 2 Sep 2020, Joe Perches wrote:
> > > > On Sun, 2020-08-30 at 20:41 +0200, Julia Lawall wrote:
> > > > > On Sun, 30 Aug 2020, Joe Perches wrote:
> > > > > > On Sun, 2020-08-30 at 17:46 +0200, Julia Lawall wrote:
> > > > > > > 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.
> > > > 
> > > > Thanks.  Hope you can get to look at that one day.
> 
> It works if you replace the addition of the new declaration by ++.  It
> seems that it is concerned that if there are multiple variables in the
> original declaration then it may be necessary to do multiple additions and
> so it doesn't do anything for a single +.  You can see this information
> with the --debug option.

Thanks, works now.

> The newlines in the generated code are also not what one would hope for.
> I will see if this can be improved.

One day.

It's not urgent for me as all the proposed changes
would need to be checked manually and the rename can
cause other alignment issues around not just the
definition changes but in code too.

e.g.:
	written = sysfs_emit(buf,
			     "foo", bar...);
becomes
	len = sysfs_emit(buf,
			     "foo", bar...);

I have other scripts that can fix both the definition
and the alignment changes done by coccinelle.

cheers, Joe

_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] transform oddity / bug ?

[Julia Lawall]

On Thu, 3 Sep 2020, Joe Perches wrote:

> On Thu, 2020-09-03 at 17:14 +0200, Julia Lawall wrote:
> > On Wed, 2 Sep 2020, Joe Perches wrote:
> > > On Wed, 2020-09-02 at 22:46 +0200, Julia Lawall wrote:
> > > > On Wed, 2 Sep 2020, Joe Perches wrote:
> > > > > On Sun, 2020-08-30 at 20:41 +0200, Julia Lawall wrote:
> > > > > > On Sun, 30 Aug 2020, Joe Perches wrote:
> > > > > > > On Sun, 2020-08-30 at 17:46 +0200, Julia Lawall wrote:
> > > > > > > > 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.
> > > > >
> > > > > Thanks.  Hope you can get to look at that one day.
> >
> > It works if you replace the addition of the new declaration by ++.  It
> > seems that it is concerned that if there are multiple variables in the
> > original declaration then it may be necessary to do multiple additions and
> > so it doesn't do anything for a single +.  You can see this information
> > with the --debug option.
>
> Thanks, works now.
>
> > The newlines in the generated code are also not what one would hope for.
> > I will see if this can be improved.
>
> One day.
>
> It's not urgent for me as all the proposed changes
> would need to be checked manually and the rename can
> cause other alignment issues around not just the
> definition changes but in code too.
>
> e.g.:
> 	written = sysfs_emit(buf,
> 			     "foo", bar...);
> becomes
> 	len = sysfs_emit(buf,
> 			     "foo", bar...);

If you remove and put back the argument list, perhaps with an expression
list metavariable, if you don't know how many arguments there are,
Coccinelle should clean this up for yout.

julia

>
> I have other scripts that can fix both the definition
> and the alignment changes done by coccinelle.
>
> cheers, Joe
>
>
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] transform oddity / bug ?

[Julia Lawall]

On Thu, 3 Sep 2020, Joe Perches wrote:

> On Thu, 2020-09-03 at 17:14 +0200, Julia Lawall wrote:
> > On Wed, 2 Sep 2020, Joe Perches wrote:
> > > On Wed, 2020-09-02 at 22:46 +0200, Julia Lawall wrote:
> > > > On Wed, 2 Sep 2020, Joe Perches wrote:
> > > > > On Sun, 2020-08-30 at 20:41 +0200, Julia Lawall wrote:
> > > > > > On Sun, 30 Aug 2020, Joe Perches wrote:
> > > > > > > On Sun, 2020-08-30 at 17:46 +0200, Julia Lawall wrote:
> > > > > > > > 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.
> > > > >
> > > > > Thanks.  Hope you can get to look at that one day.
> >
> > It works if you replace the addition of the new declaration by ++.  It
> > seems that it is concerned that if there are multiple variables in the
> > original declaration then it may be necessary to do multiple additions and
> > so it doesn't do anything for a single +.  You can see this information
> > with the --debug option.
>
> Thanks, works now.
>
> > The newlines in the generated code are also not what one would hope for.
> > I will see if this can be improved.

This should be fixed now.  You can see the test case
tests/type_and_var.{cocci,c,res}.

julia
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci