[Cocci] const in parameter lists

[Cocci] const in parameter lists

[Christoph Böhmwalder]

Hi,

I'm having trouble removing const arguments from a function. I think this issue
is best explained in code:

$ cat repro.c
f(int x,
  const unsigned a,
  const char *y)
{
}

$ cat repro.cocci
@@
identifier a;
@@
f(...
- , unsigned a
+ , unsigned b
,...)
{
...
}

$ spatch --sp-file repro.cocci --very-quiet repro.c
--- repro.c
+++ /tmp/cocci-output-17454-200d33-repro.c
@@ -1,5 +1,5 @@
-f(int x,
-  const unsigned a,
+f(int x
+  const, unsigned b,
   const char *y)
 {
 }


When I try to remove some parameter(s) in the middle (in my real use case
I'm trying to remove 2) the 'const' ends up in weird places.

Obviously I've tried (with and without disable optional_qualifier):

    f(...
    - , const unsigned a
    + , const unsigned b
    ,...)

but that gives:

    minus: parse error:
      File "repro.cocci", line 5, column 19, charpos = 45
      around = 'a',
      whole content = - , const unsigned a

How can I tell coccinelle that I want a const here?

Thanks,
--
Christoph Böhmwalder
LINBIT | Keeping the Digital World Running
DRBD HA —  Disaster Recovery — Software defined Storage
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] const in parameter lists

[Julia Lawall]

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



On Thu, 12 Sep 2019, Christoph Böhmwalder wrote:

> Hi,
>
> I'm having trouble removing const arguments from a function. I think this issue
> is best explained in code:
>
> $ cat repro.c
> f(int x,
>   const unsigned a,
>   const char *y)
> {
> }
>
> $ cat repro.cocci
> @@
> identifier a;
> @@
> f(...
> - , unsigned a
> + , unsigned b
> ,...)
> {
> ...
> }
>
> $ spatch --sp-file repro.cocci --very-quiet repro.c
> --- repro.c
> +++ /tmp/cocci-output-17454-200d33-repro.c
> @@ -1,5 +1,5 @@
> -f(int x,
> -  const unsigned a,
> +f(int x
> +  const, unsigned b,
>    const char *y)
>  {
>  }
>
>
> When I try to remove some parameter(s) in the middle (in my real use case
> I'm trying to remove 2) the 'const' ends up in weird places.
>
> Obviously I've tried (with and without disable optional_qualifier):
>
>     f(...
>     - , const unsigned a
>     + , const unsigned b
>     ,...)
>
> but that gives:
>
>     minus: parse error:
>       File "repro.cocci", line 5, column 19, charpos = 45
>       around = 'a',
>       whole content = - , const unsigned a
>
> How can I tell coccinelle that I want a const here?


Sorry, it's not clear to me what you want to do.  Do you want to verify
that there is a const before renaming the parameter?  Could you do

const unsigned
-a
+b

?

julia

>
> Thanks,
> --
> Christoph Böhmwalder
> LINBIT | Keeping the Digital World Running
> DRBD HA —  Disaster Recovery — Software defined Storage
> _______________________________________________
> Cocci mailing list
> Cocci@systeme.lip6.fr
> https://systeme.lip6.fr/mailman/listinfo/cocci
>

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

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

Re: [Cocci] const in parameter lists

[Christoph Böhmwalder]

On Thu, Sep 12, 2019 at 01:31:52PM +0200, Julia Lawall wrote:
> 
> Sorry, it's not clear to me what you want to do.  Do you want to verify
> that there is a const before renaming the parameter?  Could you do
> 
> const unsigned
> -a
> +b
> 
> ?
> 
> julia

I'll try and outline what I'm actually trying to do.

Here's what I have:

int drbd_submit_peer_request(struct drbd_device *device,
			     struct drbd_peer_request *peer_req,
			     const unsigned op, const unsigned op_flags,
			     const int fault_type)
{
// ...
}

This is what I want:

int drbd_submit_peer_request(struct drbd_device *device,
			     struct drbd_peer_request *peer_req,
			     const unsigned rw,
			     const int fault_type)
{
// ...
}

And this is my cocci patch:

@@
identifier op, op_flags;
struct bio *b;
@@
drbd_submit_peer_request(...
-    , const unsigned op, const unsigned op_flags
+    , const unsigned rw
 ,...)
{
...
}

This gives an error:

minus: parse error:
  File "drbd/drbd-kernel-compat/cocci/req_write__yes_present.cocci", line 35, column 22, charpos = 548
  around = 'op',
  whole content = -    , const unsigned op, const unsigned op_flags


Doing it without the consts yields this:

 int drbd_submit_peer_request(struct drbd_device *device,
-                            struct drbd_peer_request *peer_req,
-                            const unsigned op, const unsigned op_flags,
+                            struct drbd_peer_request *peer_reqconst,
+                            unsigned rw,
                             const int fault_type)
 {

Notice the stray "const" behind "peer_req".

So -- to answer your question -- my priority here is to get the code
compiling; I don't actually care about any of the consts. Best case
scenario would be to have it remove whether or not the consts are there
and always add the new parameter with a const.

--
Christoph Böhmwalder
LINBIT | Keeping the Digital World Running
DRBD HA —  Disaster Recovery — Software defined Storage
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] const in parameter lists

[Julia Lawall]

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



On Thu, 12 Sep 2019, Christoph Böhmwalder wrote:

> On Thu, Sep 12, 2019 at 01:31:52PM +0200, Julia Lawall wrote:
> >
> > Sorry, it's not clear to me what you want to do.  Do you want to verify
> > that there is a const before renaming the parameter?  Could you do
> >
> > const unsigned
> > -a
> > +b
> >
> > ?
> >
> > julia
>
> I'll try and outline what I'm actually trying to do.
>
> Here's what I have:
>
> int drbd_submit_peer_request(struct drbd_device *device,
> 			     struct drbd_peer_request *peer_req,
> 			     const unsigned op, const unsigned op_flags,
> 			     const int fault_type)
> {
> // ...
> }
>
> This is what I want:
>
> int drbd_submit_peer_request(struct drbd_device *device,
> 			     struct drbd_peer_request *peer_req,
> 			     const unsigned rw,
> 			     const int fault_type)
> {
> // ...
> }
>
> And this is my cocci patch:
>
> @@
> identifier op, op_flags;
> struct bio *b;
> @@
> drbd_submit_peer_request(...
> -    , const unsigned op, const unsigned op_flags
> +    , const unsigned rw
>  ,...)
> {
> ...
> }
>
> This gives an error:
>
> minus: parse error:
>   File "drbd/drbd-kernel-compat/cocci/req_write__yes_present.cocci", line 35, column 22, charpos = 548
>   around = 'op',
>   whole content = -    , const unsigned op, const unsigned op_flags
>
>
> Doing it without the consts yields this:
>
>  int drbd_submit_peer_request(struct drbd_device *device,
> -                            struct drbd_peer_request *peer_req,
> -                            const unsigned op, const unsigned op_flags,
> +                            struct drbd_peer_request *peer_reqconst,
> +                            unsigned rw,
>                              const int fault_type)
>  {
>
> Notice the stray "const" behind "peer_req".
>
> So -- to answer your question -- my priority here is to get the code
> compiling; I don't actually care about any of the consts. Best case
> scenario would be to have it remove whether or not the consts are there
> and always add the new parameter with a const.

How about the following?

@@
identifier op, op_flags;
struct bio *b;
type T;
@@
T drbd_submit_peer_request(...,
-       unsigned op
+       unsigned rw
        ,
-       unsigned op_flags,
        ...)
{
...
}

It doesn't add a const on rw, though.  And it doesn't allow using the
const to figure out where to match.  I will have to look into why the
consts aren't being parsed properly.

julia

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

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

Re: [Cocci] const in parameter lists

[Markus Elfring]

> drbd_submit_peer_request(...
> -    , const unsigned op, const unsigned op_flags
> +    , const unsigned rw
>  ,...)
> {
> ...
> }

How do you think about the following SmPL change specification?

 drbd_submit_peer_request(...,
-                         const unsigned op,
                          const unsigned
-                                        op_flags
+                                        rw
                          , ...)
 {
 ...
 }


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

Re: [Cocci] const in parameter lists

[Julia Lawall]

On Thu, 12 Sep 2019, Markus Elfring wrote:

> > drbd_submit_peer_request(...
> > -    , const unsigned op, const unsigned op_flags
> > +    , const unsigned rw
> >  ,...)
> > {
> > ...
> > }
>
> How do you think about the following SmPL change specification?
>
>  drbd_submit_peer_request(...,
> -                         const unsigned op,
>                           const unsigned
> -                                        op_flags
> +                                        rw
>                           , ...)
>  {
>  ...
>  }

It would really be helpful if you would actually test things before
proposing them.  This produces the same parse error as the other options.

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

Re: [Cocci] const in parameter lists

[Markus Elfring]

> It would really be helpful if you would actually test things before
> proposing them.

Should the suggested SmPL approach work also finally?


> This produces the same parse error as the other options.

Will the software situation improve this use case?

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

Re: [Cocci] const in parameter lists

[Julia Lawall]

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



On Thu, 12 Sep 2019, Christoph Böhmwalder wrote:

> On Thu, Sep 12, 2019 at 01:31:52PM +0200, Julia Lawall wrote:
> >
> > Sorry, it's not clear to me what you want to do.  Do you want to verify
> > that there is a const before renaming the parameter?  Could you do
> >
> > const unsigned
> > -a
> > +b
> >
> > ?
> >
> > julia
>
> I'll try and outline what I'm actually trying to do.
>
> Here's what I have:
>
> int drbd_submit_peer_request(struct drbd_device *device,
> 			     struct drbd_peer_request *peer_req,
> 			     const unsigned op, const unsigned op_flags,
> 			     const int fault_type)
> {
> // ...
> }
>
> This is what I want:
>
> int drbd_submit_peer_request(struct drbd_device *device,
> 			     struct drbd_peer_request *peer_req,
> 			     const unsigned rw,
> 			     const int fault_type)
> {
> // ...
> }
>
> And this is my cocci patch:
>
> @@
> identifier op, op_flags;
> struct bio *b;
> @@
> drbd_submit_peer_request(...
> -    , const unsigned op, const unsigned op_flags
> +    , const unsigned rw

Const was not supported in front of signed or unsigned (as opposed to
signed int or unsigned int).  I have fixed this, but am not able to
update github due to a build issue.  But your semantic patch will work if
you put unsigned int in each case.  It will not require the int to be
present in the C code.

julia

>  ,...)
> {
> ...
> }
>
> This gives an error:
>
> minus: parse error:
>   File "drbd/drbd-kernel-compat/cocci/req_write__yes_present.cocci", line 35, column 22, charpos = 548
>   around = 'op',
>   whole content = -    , const unsigned op, const unsigned op_flags
>
>
> Doing it without the consts yields this:
>
>  int drbd_submit_peer_request(struct drbd_device *device,
> -                            struct drbd_peer_request *peer_req,
> -                            const unsigned op, const unsigned op_flags,
> +                            struct drbd_peer_request *peer_reqconst,
> +                            unsigned rw,
>                              const int fault_type)
>  {
>
> Notice the stray "const" behind "peer_req".
>
> So -- to answer your question -- my priority here is to get the code
> compiling; I don't actually care about any of the consts. Best case
> scenario would be to have it remove whether or not the consts are there
> and always add the new parameter with a const.
>
> --
> Christoph Böhmwalder
> LINBIT | Keeping the Digital World Running
> DRBD HA —  Disaster Recovery — Software defined Storage
>

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

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

Re: [Cocci] const in parameter lists

[Markus Elfring]

> Const was not supported in front of signed or unsigned (as opposed to
> signed int or unsigned int).

Thanks for this explanation.


> I have fixed this,

Will the software extension trigger any consequences for feature requests
like “Support for varying order of type qualifiers”?
https://github.com/coccinelle/coccinelle/issues/49


> but am not able toupdate github due to a build issue.

Can any contributors help any more here?

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