[Cocci] moving const instead of not matching?

[Cocci] moving const instead of not matching?

[Johannes Berg]

I was trying to do something like this:

@@
typedef u8;
expression D, T, S2;
@@
(
 memcpy(
-(u8 *)(D)
+D
 ,
-(u8 *)(T)
+T
 , S2)
|
 memcpy(
-(u8 *)(D)
+D
 ,
-(T)
+T
 , S2)
|
 memcpy(
-(D)
+D
 ,
-(u8 *)(T)
+T
 , S2)
)


but then I ran across a place that had a const, and the result was
rather unexpected:

$ spatch --sp-file /tmp/test3.spatch /tmp/test.c
init_defs_builtins: /usr/lib64/coccinelle/standard.h
HANDLING: /tmp/test.c
diff = 
--- /tmp/test.c
+++ /tmp/cocci-output-8879-8411db-test.c
@@ -1,4 +1,4 @@
 int main()
 {
-	memcpy(dst, (const u8 *)src, sz);
+	memcpy(dst, src const, sz);
 }


Why is the const being moved there, rather than not matching?

spatch version 1.0.6.

johannes

[Cocci] moving const instead of not matching?

[Julia Lawall]

On Tue, 18 Sep 2018, Johannes Berg wrote:

> I was trying to do something like this:
>
> @@
> typedef u8;
> expression D, T, S2;
> @@
> (
>  memcpy(
> -(u8 *)(D)
> +D
>  ,
> -(u8 *)(T)
> +T
>  , S2)
> |
>  memcpy(
> -(u8 *)(D)
> +D
>  ,
> -(T)
> +T
>  , S2)
> |
>  memcpy(
> -(D)
> +D
>  ,
> -(u8 *)(T)
> +T
>  , S2)
> )
>
>
> but then I ran across a place that had a const, and the result was
> rather unexpected:
>
> $ spatch --sp-file /tmp/test3.spatch /tmp/test.c
> init_defs_builtins: /usr/lib64/coccinelle/standard.h
> HANDLING: /tmp/test.c
> diff =
> --- /tmp/test.c
> +++ /tmp/cocci-output-8879-8411db-test.c
> @@ -1,4 +1,4 @@
>  int main()
>  {
> -	memcpy(dst, (const u8 *)src, sz);
> +	memcpy(dst, src const, sz);
>  }
>
>
> Why is the const being moved there, rather than not matching?

To get it to not match this case, put disable optional_qualifier in the
rule header.  There is an isomorphism that allows it to not care about
whether things like const or volatile are present or not, but apparently
it is not deleting it as it should be.

If you want to match the const case, you can make some other rules with
const explicitly.

> spatch version 1.0.6.

You may want to upgrade, although it's unrelated to this issue.

julia

[Cocci] moving const instead of not matching?

[Johannes Berg]

On Tue, 2018-09-18 at 12:41 +0200, Julia Lawall wrote:
> > but then I ran across a place that had a const, and the result was
> > rather unexpected:
> > 
> > $ spatch --sp-file /tmp/test3.spatch /tmp/test.c
> > init_defs_builtins: /usr/lib64/coccinelle/standard.h
> > HANDLING: /tmp/test.c
> > diff =
> > --- /tmp/test.c
> > +++ /tmp/cocci-output-8879-8411db-test.c
> > @@ -1,4 +1,4 @@
> >  int main()
> >  {
> > -	memcpy(dst, (const u8 *)src, sz);
> > +	memcpy(dst, src const, sz);
> >  }
> > 
> > 
> > Why is the const being moved there, rather than not matching?
> 
> To get it to not match this case, put disable optional_qualifier in the
> rule header.

Ok, I can try that.

>   There is an isomorphism that allows it to not care about
> whether things like const or volatile are present or not, but apparently
> it is not deleting it as it should be.

Still seems like a bug?

> If you want to match the const case, you can make some other rules with
> const explicitly.

Right. I tried to put that before this one, but somehow it still
resulted in the same. Perhaps if I disable optional_qualifier for both
it'll work.

> > spatch version 1.0.6.
> 
> You may want to upgrade, although it's unrelated to this issue.

Yeah, I'm (obviously) aware, but with all the version number issues with
1.0.7 I haven't gotten around to it :)

johannes

[Cocci] moving const instead of not matching?

[Julia Lawall]

On Tue, 18 Sep 2018, Johannes Berg wrote:

> On Tue, 2018-09-18 at 12:41 +0200, Julia Lawall wrote:
> > > but then I ran across a place that had a const, and the result was
> > > rather unexpected:
> > >
> > > $ spatch --sp-file /tmp/test3.spatch /tmp/test.c
> > > init_defs_builtins: /usr/lib64/coccinelle/standard.h
> > > HANDLING: /tmp/test.c
> > > diff =
> > > --- /tmp/test.c
> > > +++ /tmp/cocci-output-8879-8411db-test.c
> > > @@ -1,4 +1,4 @@
> > >  int main()
> > >  {
> > > -	memcpy(dst, (const u8 *)src, sz);
> > > +	memcpy(dst, src const, sz);
> > >  }
> > >
> > >
> > > Why is the const being moved there, rather than not matching?
> >
> > To get it to not match this case, put disable optional_qualifier in the
> > rule header.
>
> Ok, I can try that.
>
> >   There is an isomorphism that allows it to not care about
> > whether things like const or volatile are present or not, but apparently
> > it is not deleting it as it should be.
>
> Still seems like a bug?


Yes, definitely a bug.  Thanks for the report.

julia

>
> > If you want to match the const case, you can make some other rules with
> > const explicitly.
>
> Right. I tried to put that before this one, but somehow it still
> resulted in the same. Perhaps if I disable optional_qualifier for both
> it'll work.
>
> > > spatch version 1.0.6.
> >
> > You may want to upgrade, although it's unrelated to this issue.
>
> Yeah, I'm (obviously) aware, but with all the version number issues with
> 1.0.7 I haven't gotten around to it :)
>
> johannes
>