All of lore.kernel.org
 help / color / mirror / Atom feed
* [Cocci] how to make substitutions at the end of the function, vs. the end of each block ?
@ 2021-07-22 19:24 Andrew 👽  Yourtchenko
  2021-07-22 20:11 ` Julia Lawall
  0 siblings, 1 reply; 7+ messages in thread
From: Andrew 👽  Yourtchenko @ 2021-07-22 19:24 UTC (permalink / raw)
  To: cocci

Hi all,

I work on the VPP project (http://fd.io/ - open source software
dataplane), and  tried to use coccinelle to make a relatively
non-trivial change
as in the mail https://lists.fd.io/g/vpp-dev/message/17532 - it seemed
to be a very good candidate - boring enough to be painful to do by
hand, complex enough to make sed inadequate for it.

I came up with this semantic patch:


@ detect_func @
identifier CLI_FN, AVM, AIN, ACMD;
fresh identifier LAIN = "line_" ## AIN;

statement S1;

typedef clib_error_t, vlib_main_t, unformat_input_t, vlib_cli_command_t;
@@

static clib_error_t *CLI_FN (vlib_main_t * AVM, unformat_input_t *
AIN, vlib_cli_command_t * ACMD)
{
+ clib_error_t *e = 0;
+  unformat_input_t *LAIN;
...
+ if (!unformat_user (AIN, unformat_line_input, LAIN)) {
+    return 0;
+ }
+
-  while (unformat_check_input (AIN) != UNFORMAT_END_OF_INPUT)
+ while (unformat_check_input (LAIN) != UNFORMAT_END_OF_INPUT)
S1
<...
- return ERR;
+ e = ERR;
+ goto done;
...>
+done:
+ unformat_free(LAIN);
+ return e;
}


I attempt to run it on this test file:

ubuntu@vpp-dev:~$ cat ~/test.c
static clib_error_t *
syn_filter_enable_disable_command_fn (vlib_main_t * vm,
                                      unformat_input_t * input,
                                      vlib_cli_command_t * cmd)
{
  vnet_main_t *vnm = vnet_get_main ();
  u32 sw_if_index = ~0;
  int enable_disable = 1;
  int rv;

  while (unformat_check_input (input) != UNFORMAT_END_OF_INPUT)
    {
      if (unformat (input, "disable"))
        enable_disable = 0;
      else if (unformat (input, "%U", unformat_vnet_sw_interface,
                         vnm, &sw_if_index))
        ;
      else
        break;
    }

  if (sw_if_index == ~0)
    return clib_error_return (0, "Please specify an interface...");

  rv = syn_filter_enable_disable (sw_if_index, enable_disable);

  switch (rv)
    {
    case 0:
      break;

    case VNET_API_ERROR_INVALID_SW_IF_INDEX:
      return clib_error_return
        (0, "Invalid interface, only works on physical ports");
      break;

    case VNET_API_ERROR_UNIMPLEMENTED:
      return clib_error_return (0,
                                "Device driver doesn't support redirection");
      break;

    case VNET_API_ERROR_INVALID_VALUE:
      return clib_error_return (0, "feature arc not found");

    case VNET_API_ERROR_INVALID_VALUE_2:
      return clib_error_return (0, "feature node not found");

    default:
      return clib_error_return (0, "syn_filter_enable_disable returned %d",
                                rv);
    }
  return 0;
}
ubuntu@vpp-dev:~$


However, when I run it, the "done: " label, etc. gets inserted twice:

ubuntu@vpp-dev:~$ spatch --sp-file /tmp/rules.sp
--allow-inconsistent-paths  ~/test.c
init_defs_builtins: /usr/bin/../lib/coccinelle/standard.h
HANDLING: /home/ubuntu/test.c
diff =
--- /home/ubuntu/test.c
+++ /tmp/cocci-output-56896-8f35c5-test.c
@@ -3,12 +3,18 @@ syn_filter_enable_disable_command_fn (vl
                                      unformat_input_t * input,
                                      vlib_cli_command_t * cmd)
 {
+  clib_error_t *e = 0;
+  unformat_input_t *line_input;
   vnet_main_t *vnm = vnet_get_main ();
   u32 sw_if_index = ~0;
   int enable_disable = 1;
   int rv;

-  while (unformat_check_input (input) != UNFORMAT_END_OF_INPUT)
+  if (!unformat_user(input, unformat_line_input, line_input)) {
+      return 0;
+  }
+
+while (unformat_check_input(line_input) != UNFORMAT_END_OF_INPUT)
     {
       if (unformat (input, "disable"))
        enable_disable = 0;
@@ -48,6 +54,12 @@ syn_filter_enable_disable_command_fn (vl
     default:
       return clib_error_return (0, "syn_filter_enable_disable returned %d",
                                rv);
-    }
+    done:
+      unformat_free(line_input);
+      return e;
+  }
   return 0;
+done:
+  unformat_free(line_input);
+  return e;
 }
ubuntu@vpp-dev:~$


I get a feeling I am missing something fundamental - but RTFM did not
help much... What am I doing wrong / missing ?
Could anyone please nudge me in the correct direction ?

Thanks a lot!

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Cocci] how to make substitutions at the end of the function, vs. the end of each block ?
  2021-07-22 19:24 [Cocci] how to make substitutions at the end of the function, vs. the end of each block ? Andrew 👽  Yourtchenko
@ 2021-07-22 20:11 ` Julia Lawall
  2021-07-22 21:21   ` Andrew 👽  Yourtchenko
  0 siblings, 1 reply; 7+ messages in thread
From: Julia Lawall @ 2021-07-22 20:11 UTC (permalink / raw)
  To: Andrew 👽  Yourtchenko; +Cc: cocci

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



On Thu, 22 Jul 2021, Andrew 👽  Yourtchenko wrote:

> Hi all,
>
> I work on the VPP project (http://fd.io/ - open source software
> dataplane), and  tried to use coccinelle to make a relatively
> non-trivial change
> as in the mail https://lists.fd.io/g/vpp-dev/message/17532 - it seemed
> to be a very good candidate - boring enough to be painful to do by
> hand, complex enough to make sed inadequate for it.

Thanks for trying Coccinelle :)

>
> I came up with this semantic patch:
>
>
> @ detect_func @
> identifier CLI_FN, AVM, AIN, ACMD;
> fresh identifier LAIN = "line_" ## AIN;
>
> statement S1;
>
> typedef clib_error_t, vlib_main_t, unformat_input_t, vlib_cli_command_t;
> @@
>
> static clib_error_t *CLI_FN (vlib_main_t * AVM, unformat_input_t *
> AIN, vlib_cli_command_t * ACMD)
> {
> + clib_error_t *e = 0;
> +  unformat_input_t *LAIN;
> ...
> + if (!unformat_user (AIN, unformat_line_input, LAIN)) {
> +    return 0;
> + }
> +
> -  while (unformat_check_input (AIN) != UNFORMAT_END_OF_INPUT)
> + while (unformat_check_input (LAIN) != UNFORMAT_END_OF_INPUT)
> S1
> <...
> - return ERR;
> + e = ERR;
> + goto done;
> ...>
> +done:
> + unformat_free(LAIN);
> + return e;
> }

The problem has to do with the fact that Coccinelle is actually oriented
around control-flow graphs. So it doesn't know which end of a control-flow
path is actually the end of the function.

You can try adjusting the line <... above as follows:

<... when != true exp
     when exists

exp should be declared as an expression metavariable.  The when != true
thing means that the path cannot cross a true branch across a test of an
expression that matches exp (ie any expression).  The when exists means
that the paths through this region of code are considered individually.

I'm not certain that this will work in every case.  It will be necessary
to check the results carefully.

Another possible hack is to first replace every return under and if,
while, etc by something else, and then rewrite all of the returns in a
third rule afterwards.  This is pretty ugly, but may be more reliable.

julia

>
> I attempt to run it on this test file:
>
> ubuntu@vpp-dev:~$ cat ~/test.c
> static clib_error_t *
> syn_filter_enable_disable_command_fn (vlib_main_t * vm,
>                                       unformat_input_t * input,
>                                       vlib_cli_command_t * cmd)
> {
>   vnet_main_t *vnm = vnet_get_main ();
>   u32 sw_if_index = ~0;
>   int enable_disable = 1;
>   int rv;
>
>   while (unformat_check_input (input) != UNFORMAT_END_OF_INPUT)
>     {
>       if (unformat (input, "disable"))
>         enable_disable = 0;
>       else if (unformat (input, "%U", unformat_vnet_sw_interface,
>                          vnm, &sw_if_index))
>         ;
>       else
>         break;
>     }
>
>   if (sw_if_index == ~0)
>     return clib_error_return (0, "Please specify an interface...");
>
>   rv = syn_filter_enable_disable (sw_if_index, enable_disable);
>
>   switch (rv)
>     {
>     case 0:
>       break;
>
>     case VNET_API_ERROR_INVALID_SW_IF_INDEX:
>       return clib_error_return
>         (0, "Invalid interface, only works on physical ports");
>       break;
>
>     case VNET_API_ERROR_UNIMPLEMENTED:
>       return clib_error_return (0,
>                                 "Device driver doesn't support redirection");
>       break;
>
>     case VNET_API_ERROR_INVALID_VALUE:
>       return clib_error_return (0, "feature arc not found");
>
>     case VNET_API_ERROR_INVALID_VALUE_2:
>       return clib_error_return (0, "feature node not found");
>
>     default:
>       return clib_error_return (0, "syn_filter_enable_disable returned %d",
>                                 rv);
>     }
>   return 0;
> }
> ubuntu@vpp-dev:~$
>
>
> However, when I run it, the "done: " label, etc. gets inserted twice:
>
> ubuntu@vpp-dev:~$ spatch --sp-file /tmp/rules.sp
> --allow-inconsistent-paths  ~/test.c
> init_defs_builtins: /usr/bin/../lib/coccinelle/standard.h
> HANDLING: /home/ubuntu/test.c
> diff =
> --- /home/ubuntu/test.c
> +++ /tmp/cocci-output-56896-8f35c5-test.c
> @@ -3,12 +3,18 @@ syn_filter_enable_disable_command_fn (vl
>                                       unformat_input_t * input,
>                                       vlib_cli_command_t * cmd)
>  {
> +  clib_error_t *e = 0;
> +  unformat_input_t *line_input;
>    vnet_main_t *vnm = vnet_get_main ();
>    u32 sw_if_index = ~0;
>    int enable_disable = 1;
>    int rv;
>
> -  while (unformat_check_input (input) != UNFORMAT_END_OF_INPUT)
> +  if (!unformat_user(input, unformat_line_input, line_input)) {
> +      return 0;
> +  }
> +
> +while (unformat_check_input(line_input) != UNFORMAT_END_OF_INPUT)
>      {
>        if (unformat (input, "disable"))
>         enable_disable = 0;
> @@ -48,6 +54,12 @@ syn_filter_enable_disable_command_fn (vl
>      default:
>        return clib_error_return (0, "syn_filter_enable_disable returned %d",
>                                 rv);
> -    }
> +    done:
> +      unformat_free(line_input);
> +      return e;
> +  }
>    return 0;
> +done:
> +  unformat_free(line_input);
> +  return e;
>  }
> ubuntu@vpp-dev:~$
>
>
> I get a feeling I am missing something fundamental - but RTFM did not
> help much... What am I doing wrong / missing ?
> Could anyone please nudge me in the correct direction ?
>
> Thanks a lot!
>
> --a
> _______________________________________________
> 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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Cocci] how to make substitutions at the end of the function, vs. the end of each block ?
  2021-07-22 20:11 ` Julia Lawall
@ 2021-07-22 21:21   ` Andrew 👽  Yourtchenko
  2021-07-23 19:35     ` Julia Lawall
                       ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Andrew 👽  Yourtchenko @ 2021-07-22 21:21 UTC (permalink / raw)
  To: Julia Lawall; +Cc: cocci

Hi Julia,

thanks a lot for such a quick reply! :)

I tried this:

yourtch@ayourtch-lnx:~/cocci$ diff -c patch-old.cocci patch-new.cocci
*** patch-old.cocci	2021-07-22 22:41:19.516957878 +0200
--- patch-new.cocci	2021-07-22 22:41:52.625184341 +0200
***************
*** 3,8 ****
--- 3,9 ----
  fresh identifier LAIN = "line_" ## AIN;

  statement S1;
+ expression exp;

  typedef clib_error_t, vlib_main_t, unformat_input_t, vlib_cli_command_t;
  @@
***************
*** 20,30 ****
  -  while (unformat_check_input (AIN) != UNFORMAT_END_OF_INPUT)
  + while (unformat_check_input (LAIN) != UNFORMAT_END_OF_INPUT)
  S1
! <...
  - return ERR;
  + e = ERR;
  + goto done;
! ...>
  +done:
  + unformat_free(LAIN);
  + return e;
--- 21,32 ----
  -  while (unformat_check_input (AIN) != UNFORMAT_END_OF_INPUT)
  + while (unformat_check_input (LAIN) != UNFORMAT_END_OF_INPUT)
  S1
! <... when != true exp
!      when exists
  - return ERR;
  + e = ERR;
  + goto done;
! ...>
  +done:
  + unformat_free(LAIN);
  + return e;
ayourtch@ayourtch-lnx:~/cocci$

And the result was the same... but I am now seeing another bug in my
original patch - *none* of the return statements within the switch()
statement are replaced as well... and somewhat start to get the idea
of why the idea of using the --allow-inconsistent-paths may not have
been a good one :)

 spatch --sp-file patch-new.cocci file.c
init_defs_builtins: /usr/bin/../lib/coccinelle/standard.h
HANDLING: file.c
detect_func: node 84: }1[1] in syn_filter_enable_disable_command_fn
reachable by inconsistent control-flow paths

I am not sure I grok the }1[1] notation well, but my hunch is that it
gets lost somewhere around that switch() statement, and I just told it
"continue anyway", so it is hard to complain the result may be
unexpected :-)

Debugging it a bit more, though I think i found at least the issue of
the "inconsistent paths" - there is a "return" inside a case
statement, followed by break. I copypasted the example file from a
longer production code and didn't check. That break is correctly
detected as dead code, but seems to make the analysis engine give up ?
There are actually two of those "return; break" sequences. After I
remove the "break", I can perform the patching without the
"inconsistent control flow path" error, but the result is exactly the
same as in the beginning...

I missed the fact that the "return ERR" were actually not replaced on
this example at all, so naturally trying the "hacky" approach didn't
work either... I think I am still not fully getting the relationship
between the semantic patch syntax and the code paths, i will give
another read to the docs and see if they eventually sink in... :-)

The basic examples in the doc seem to be pretty straightforward, but
the move to a more complicated cases like this seems to introduce a
mental bump, I think I just need to persist on it a bit more :-)

--a





On 7/22/21, Julia Lawall <julia.lawall@inria.fr> wrote:
>
>
> On Thu, 22 Jul 2021, Andrew 👽  Yourtchenko wrote:
>
>> Hi all,
>>
>> I work on the VPP project (http://fd.io/ - open source software
>> dataplane), and  tried to use coccinelle to make a relatively
>> non-trivial change
>> as in the mail https://lists.fd.io/g/vpp-dev/message/17532 - it seemed
>> to be a very good candidate - boring enough to be painful to do by
>> hand, complex enough to make sed inadequate for it.
>
> Thanks for trying Coccinelle :)
>
>>
>> I came up with this semantic patch:
>>
>>
>> @ detect_func @
>> identifier CLI_FN, AVM, AIN, ACMD;
>> fresh identifier LAIN = "line_" ## AIN;
>>
>> statement S1;
>>
>> typedef clib_error_t, vlib_main_t, unformat_input_t, vlib_cli_command_t;
>> @@
>>
>> static clib_error_t *CLI_FN (vlib_main_t * AVM, unformat_input_t *
>> AIN, vlib_cli_command_t * ACMD)
>> {
>> + clib_error_t *e = 0;
>> +  unformat_input_t *LAIN;
>> ...
>> + if (!unformat_user (AIN, unformat_line_input, LAIN)) {
>> +    return 0;
>> + }
>> +
>> -  while (unformat_check_input (AIN) != UNFORMAT_END_OF_INPUT)
>> + while (unformat_check_input (LAIN) != UNFORMAT_END_OF_INPUT)
>> S1
>> <...
>> - return ERR;
>> + e = ERR;
>> + goto done;
>> ...>
>> +done:
>> + unformat_free(LAIN);
>> + return e;
>> }
>
> The problem has to do with the fact that Coccinelle is actually oriented
> around control-flow graphs. So it doesn't know which end of a control-flow
> path is actually the end of the function.
>
> You can try adjusting the line <... above as follows:
>
> <... when != true exp
>      when exists
>
> exp should be declared as an expression metavariable.  The when != true
> thing means that the path cannot cross a true branch across a test of an
> expression that matches exp (ie any expression).  The when exists means
> that the paths through this region of code are considered individually.
>
> I'm not certain that this will work in every case.  It will be necessary
> to check the results carefully.
>
> Another possible hack is to first replace every return under and if,
> while, etc by something else, and then rewrite all of the returns in a
> third rule afterwards.  This is pretty ugly, but may be more reliable.
>
> julia
>
>>
>> I attempt to run it on this test file:
>>
>> ubuntu@vpp-dev:~$ cat ~/test.c
>> static clib_error_t *
>> syn_filter_enable_disable_command_fn (vlib_main_t * vm,
>>                                       unformat_input_t * input,
>>                                       vlib_cli_command_t * cmd)
>> {
>>   vnet_main_t *vnm = vnet_get_main ();
>>   u32 sw_if_index = ~0;
>>   int enable_disable = 1;
>>   int rv;
>>
>>   while (unformat_check_input (input) != UNFORMAT_END_OF_INPUT)
>>     {
>>       if (unformat (input, "disable"))
>>         enable_disable = 0;
>>       else if (unformat (input, "%U", unformat_vnet_sw_interface,
>>                          vnm, &sw_if_index))
>>         ;
>>       else
>>         break;
>>     }
>>
>>   if (sw_if_index == ~0)
>>     return clib_error_return (0, "Please specify an interface...");
>>
>>   rv = syn_filter_enable_disable (sw_if_index, enable_disable);
>>
>>   switch (rv)
>>     {
>>     case 0:
>>       break;
>>
>>     case VNET_API_ERROR_INVALID_SW_IF_INDEX:
>>       return clib_error_return
>>         (0, "Invalid interface, only works on physical ports");
>>       break;
>>
>>     case VNET_API_ERROR_UNIMPLEMENTED:
>>       return clib_error_return (0,
>>                                 "Device driver doesn't support
>> redirection");
>>       break;
>>
>>     case VNET_API_ERROR_INVALID_VALUE:
>>       return clib_error_return (0, "feature arc not found");
>>
>>     case VNET_API_ERROR_INVALID_VALUE_2:
>>       return clib_error_return (0, "feature node not found");
>>
>>     default:
>>       return clib_error_return (0, "syn_filter_enable_disable returned
>> %d",
>>                                 rv);
>>     }
>>   return 0;
>> }
>> ubuntu@vpp-dev:~$
>>
>>
>> However, when I run it, the "done: " label, etc. gets inserted twice:
>>
>> ubuntu@vpp-dev:~$ spatch --sp-file /tmp/rules.sp
>> --allow-inconsistent-paths  ~/test.c
>> init_defs_builtins: /usr/bin/../lib/coccinelle/standard.h
>> HANDLING: /home/ubuntu/test.c
>> diff =
>> --- /home/ubuntu/test.c
>> +++ /tmp/cocci-output-56896-8f35c5-test.c
>> @@ -3,12 +3,18 @@ syn_filter_enable_disable_command_fn (vl
>>                                       unformat_input_t * input,
>>                                       vlib_cli_command_t * cmd)
>>  {
>> +  clib_error_t *e = 0;
>> +  unformat_input_t *line_input;
>>    vnet_main_t *vnm = vnet_get_main ();
>>    u32 sw_if_index = ~0;
>>    int enable_disable = 1;
>>    int rv;
>>
>> -  while (unformat_check_input (input) != UNFORMAT_END_OF_INPUT)
>> +  if (!unformat_user(input, unformat_line_input, line_input)) {
>> +      return 0;
>> +  }
>> +
>> +while (unformat_check_input(line_input) != UNFORMAT_END_OF_INPUT)
>>      {
>>        if (unformat (input, "disable"))
>>         enable_disable = 0;
>> @@ -48,6 +54,12 @@ syn_filter_enable_disable_command_fn (vl
>>      default:
>>        return clib_error_return (0, "syn_filter_enable_disable returned
>> %d",
>>                                 rv);
>> -    }
>> +    done:
>> +      unformat_free(line_input);
>> +      return e;
>> +  }
>>    return 0;
>> +done:
>> +  unformat_free(line_input);
>> +  return e;
>>  }
>> ubuntu@vpp-dev:~$
>>
>>
>> I get a feeling I am missing something fundamental - but RTFM did not
>> help much... What am I doing wrong / missing ?
>> Could anyone please nudge me in the correct direction ?
>>
>> Thanks a lot!
>>
>> --a
>> _______________________________________________
>> Cocci mailing list
>> Cocci@systeme.lip6.fr
>> https://systeme.lip6.fr/mailman/listinfo/cocci
>>
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Cocci] how to make substitutions at the end of the function, vs. the end of each block ?
  2021-07-22 21:21   ` Andrew 👽  Yourtchenko
@ 2021-07-23 19:35     ` Julia Lawall
  2021-07-24 14:59       ` Andrew 👽 Yourtchenko
  2021-07-23 19:40     ` Julia Lawall
  2021-07-23 20:00     ` Julia Lawall
  2 siblings, 1 reply; 7+ messages in thread
From: Julia Lawall @ 2021-07-23 19:35 UTC (permalink / raw)
  To: Andrew 👽  Yourtchenko; +Cc: cocci

> I missed the fact that the "return ERR" were actually not replaced on
> this example at all,

This is because there are no occurrences of return ERR; in your code.
Probably you expected ERR to be a metavariable, but it's not.  Actually,
fully capitalizing your metavariables is not a good idea.  Coccinelle
expects that fully capitalized things are constants, as in #define ERR -1,
and so when they appear at random places, it doesn't comment about that.
If it had been in lowercase and used as the argument of a return (or as a
function argument, right-hand side of an assignment, etc), Coccinelle
would have printed a warning wondering if you expected it to be a
metavariable.

If ERR is declared as an expression metavariable, then lots of returns are
replaced.

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Cocci] how to make substitutions at the end of the function, vs. the end of each block ?
  2021-07-22 21:21   ` Andrew 👽  Yourtchenko
  2021-07-23 19:35     ` Julia Lawall
@ 2021-07-23 19:40     ` Julia Lawall
  2021-07-23 20:00     ` Julia Lawall
  2 siblings, 0 replies; 7+ messages in thread
From: Julia Lawall @ 2021-07-23 19:40 UTC (permalink / raw)
  To: Andrew 👽  Yourtchenko; +Cc: cocci

> yourtch@ayourtch-lnx:~/cocci$ diff -c patch-old.cocci patch-new.cocci
> *** patch-old.cocci	2021-07-22 22:41:19.516957878 +0200
> --- patch-new.cocci	2021-07-22 22:41:52.625184341 +0200
> ***************
> *** 3,8 ****
> --- 3,9 ----
>   fresh identifier LAIN = "line_" ## AIN;
>
>   statement S1;
> + expression exp;
>
>   typedef clib_error_t, vlib_main_t, unformat_input_t, vlib_cli_command_t;
>   @@
> ***************
> *** 20,30 ****
>   -  while (unformat_check_input (AIN) != UNFORMAT_END_OF_INPUT)
>   + while (unformat_check_input (LAIN) != UNFORMAT_END_OF_INPUT)
>   S1
> ! <...
>   - return ERR;
>   + e = ERR;
>   + goto done;
> ! ...>
>   +done:
>   + unformat_free(LAIN);
>   + return e;
> --- 21,32 ----
>   -  while (unformat_check_input (AIN) != UNFORMAT_END_OF_INPUT)
>   + while (unformat_check_input (LAIN) != UNFORMAT_END_OF_INPUT)
>   S1
> ! <... when != true exp
> !      when exists
>   - return ERR;
>   + e = ERR;
>   + goto done;
> ! ...>
>   +done:
>   + unformat_free(LAIN);
>   + return e;
> ayourtch@ayourtch-lnx:~/cocci$
>
> And the result was the same...

My trick doesn't work because the return of interest is under a switch,
where there is no test expression that has the value true or false.  I
will see if something else can be done.

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Cocci] how to make substitutions at the end of the function, vs. the end of each block ?
  2021-07-22 21:21   ` Andrew 👽  Yourtchenko
  2021-07-23 19:35     ` Julia Lawall
  2021-07-23 19:40     ` Julia Lawall
@ 2021-07-23 20:00     ` Julia Lawall
  2 siblings, 0 replies; 7+ messages in thread
From: Julia Lawall @ 2021-07-23 20:00 UTC (permalink / raw)
  To: Andrew 👽  Yourtchenko; +Cc: cocci

Here is another attempt:

@initialize:ocaml@
@@

let check p =
  let p = List.hd p in
  p.line_end = p.current_element_line_end

@ detect_func @
identifier CLI_FN, AVM, AIN, ACMD;
fresh identifier LAIN = "line_" ## AIN;
expression ERR, exp;
statement S1;
typedef clib_error_t, vlib_main_t, unformat_input_t, vlib_cli_command_t;
@@

static clib_error_t *CLI_FN (vlib_main_t * AVM, unformat_input_t *
AIN, vlib_cli_command_t * ACMD)
{
+ clib_error_t *e = 0;
+  unformat_input_t *LAIN;
...
+ if (!unformat_user (AIN, unformat_line_input, LAIN)) {
+    return 0;
+ }
+
-  while (unformat_check_input (AIN) != UNFORMAT_END_OF_INPUT)
+ while (unformat_check_input (LAIN) != UNFORMAT_END_OF_INPUT)
S1
<...
- return ERR;
+ e = ERR;
+ goto done;
...>
}

// the following rule should rematch the function matched in the previous rule
@@
identifier detect_func.CLI_FN, detect_func.AVM, detect_func.AIN, detect_func.LAIN, detect_func.ACMD;
position p : script:ocaml() { check p }; // check that the matched position is at the end of the function
@@

static clib_error_t *CLI_FN (vlib_main_t * AVM, unformat_input_t *
AIN, vlib_cli_command_t * ACMD)
{
... when exists
+done:
+ unformat_free(LAIN);
+ return e;
}@p
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Cocci] how to make substitutions at the end of the function, vs. the end of each block ?
  2021-07-23 19:35     ` Julia Lawall
@ 2021-07-24 14:59       ` Andrew 👽 Yourtchenko
  0 siblings, 0 replies; 7+ messages in thread
From: Andrew 👽 Yourtchenko @ 2021-07-24 14:59 UTC (permalink / raw)
  To: Julia Lawall; +Cc: cocci



> On 23 Jul 2021, at 21:35, Julia Lawall <julia.lawall@inria.fr> wrote:
> 
> 
>> 
>> I missed the fact that the "return ERR" were actually not replaced on
>> this example at all,
> 
> This is because there are no occurrences of return ERR; in your code.
> Probably you expected ERR to be a metavariable, but it's not.  

I had it declared at one point declared as “expression ERR;”, so total PEBCAK here, sorry for that! 🙈


> Actually,
> fully capitalizing your metavariables is not a good idea.  Coccinelle
> expects that fully capitalized things are constants, as in #define ERR -1,
> and so when they appear at random places, it doesn't comment about that.
> If it had been in lowercase and used as the argument of a return (or as a
> function argument, right-hand side of an assignment, etc), Coccinelle
> would have printed a warning wondering if you expected it to be a
> metavariable.

Oh, thanks a lot for the tip! 

> 
> If ERR is declared as an expression metavariable, then lots of returns are
> replaced.

Excellent, thanks a lot for the education ! :)

—a

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2021-07-24 14:59 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-22 19:24 [Cocci] how to make substitutions at the end of the function, vs. the end of each block ? Andrew 👽  Yourtchenko
2021-07-22 20:11 ` Julia Lawall
2021-07-22 21:21   ` Andrew 👽  Yourtchenko
2021-07-23 19:35     ` Julia Lawall
2021-07-24 14:59       ` Andrew 👽 Yourtchenko
2021-07-23 19:40     ` Julia Lawall
2021-07-23 20:00     ` Julia Lawall

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.