Re: [Cocci] how to make substitutions at the end of the function, vs. the end of each block ?

[Julia Lawall <julia.lawall@inria.fr>]

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