[Cocci] using cocci to switch to formatted log/print function

[Cocci] using cocci to switch to formatted log/print function

[George McCollister]

Hello,

I'm working with an old code base that makes excessive use of the
following sort of construct:

char display[128];

...

snprintf(display, sizeof(display), "example log message %d", i);
log_buffer(level, strlen(display)+1, display);

I'd like to replace this mess with a single call to a function named
log_formatted(). This involves moving the format string and arguments
passed to snprintf into the log_formatted() call, removing snprintf
call and (here's where it gets a bit tricky) remove the buffer if it
isn't used for anything else.

I have this all working with the following script with the caveat that
running it on moderately complicated source files makes it never
finish (after an hour or so the spatch process crashes with a stack
overflow error).
I've tried --no-loops which seem to speed things up but complicated
source files still result in it never finishing.

@r1@
type T;
identifier disp;
expression level;
expression list prnt;
@@

{
... when any
(
T disp[...];
|
T disp[...]="";
)
<+...
  snprintf(disp, sizeof(disp), prnt);
... when != disp
- log_buffer(level, strlen(disp)+1, disp);
+ log_formatted(level, prnt);
...+>
}

// Only remove the display variable and snprintf if there are no
// other references to the variable.

@r2 depends on r1@
type r1.T;
identifier r1.disp;
expression list r1.prnt;
@@

{
... when any
(
- T disp[...];
|
- T disp[...]="";
)
<+... when != disp
- snprintf(disp, sizeof(disp), prnt);
...+>
}


Any suggestions on changes to my script that would make this work on
lengthy source files would be greatly appreciated!

Thanks,
George McCollister
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] using cocci to switch to formatted log/print function

[Julia Lawall]

On Fri, 27 Mar 2020, George McCollister wrote:

> Hello,
>
> I'm working with an old code base that makes excessive use of the
> following sort of construct:
>
> char display[128];
>
> ...
>
> snprintf(display, sizeof(display), "example log message %d", i);
> log_buffer(level, strlen(display)+1, display);
>
> I'd like to replace this mess with a single call to a function named
> log_formatted(). This involves moving the format string and arguments
> passed to snprintf into the log_formatted() call, removing snprintf
> call and (here's where it gets a bit tricky) remove the buffer if it
> isn't used for anything else.
>
> I have this all working with the following script with the caveat that
> running it on moderately complicated source files makes it never
> finish (after an hour or so the spatch process crashes with a stack
> overflow error).
> I've tried --no-loops which seem to speed things up but complicated
> source files still result in it never finishing.
>
> @r1@
> type T;
> identifier disp;
> expression level;
> expression list prnt;
> @@
>
> {
> ... when any

The above is not necessary.  YOu don't have to start the match from a
block.  You can just start it from the declaration.

On the other hand, the declaration may not be needed either.  After your
metavariable type T, you can put

local idexpression T[] disp2;

Then the match should be:

  snprintf(disp2@disp, sizeof(disp), prnt);
... when != disp
- log_buffer(level, strlen(disp)+1, disp);
+ log_formatted(level, prnt);

The notation disp2@disp will match both disp2 (idexpression of a
particular type) and disp (identifier) against the first argument of
snprintf.  This is needed because 1) you want to give a type, which
requires and idexpression, and 2) in later rules you want to change a
variable declaration, which requires an identifier.


> (
> T disp[...];
> |
> T disp[...]="";
> )
> <+...
>   snprintf(disp, sizeof(disp), prnt);
> ... when != disp
> - log_buffer(level, strlen(disp)+1, disp);
> + log_formatted(level, prnt);
> ...+>
> }
>
> // Only remove the display variable and snprintf if there are no
> // other references to the variable.
>
> @r2 depends on r1@
> type r1.T;
> identifier r1.disp;
> expression list r1.prnt;
> @@
>
> {
> ... when any
> (
> - T disp[...];
> |
> - T disp[...]="";
> )
> <+... when != disp
> - snprintf(disp, sizeof(disp), prnt);
> ...+>
> }

Here you could try considering the problem from the opposite point of
view.

@r2 exists@ // exists is what helps you drop the complexity, by needing to
type r1.T;  // find only one matching path
identifier r1.disp;;
expression list r1.prnt;
position p;
@@

(
T@p disp[...];
|
T@p disp[...]="";
)
...
snprintf(disp, sizeof(disp), prnt);

@r3@
position p != r2.p;
type r1.T;
identifier r1.disp;;
@@

(
- T@p disp[...];
|
- T@p disp[...]="";
)


Does it matter that the initial value of disp is ""?  In the proposed
first rule I have dropped that constraint.

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

Re: [Cocci] using cocci to switch to formatted log/print function

[George McCollister]

On Fri, Mar 27, 2020 at 1:37 PM Julia Lawall <julia.lawall@inria.fr> wrote:
>
>
>
> On Fri, 27 Mar 2020, George McCollister wrote:
>
> > Hello,
> >
> > I'm working with an old code base that makes excessive use of the
> > following sort of construct:
> >
> > char display[128];
> >
> > ...
> >
> > snprintf(display, sizeof(display), "example log message %d", i);
> > log_buffer(level, strlen(display)+1, display);
> >
> > I'd like to replace this mess with a single call to a function named
> > log_formatted(). This involves moving the format string and arguments
> > passed to snprintf into the log_formatted() call, removing snprintf
> > call and (here's where it gets a bit tricky) remove the buffer if it
> > isn't used for anything else.
> >
> > I have this all working with the following script with the caveat that
> > running it on moderately complicated source files makes it never
> > finish (after an hour or so the spatch process crashes with a stack
> > overflow error).
> > I've tried --no-loops which seem to speed things up but complicated
> > source files still result in it never finishing.
> >
> > @r1@
> > type T;
> > identifier disp;
> > expression level;
> > expression list prnt;
> > @@
> >
> > {
> > ... when any
>
> The above is not necessary.  YOu don't have to start the match from a
> block.  You can just start it from the declaration.
>
> On the other hand, the declaration may not be needed either.  After your
> metavariable type T, you can put
>
> local idexpression T[] disp2;
>
> Then the match should be:
>
>   snprintf(disp2@disp, sizeof(disp), prnt);
> ... when != disp
> - log_buffer(level, strlen(disp)+1, disp);
> + log_formatted(level, prnt);
>
> The notation disp2@disp will match both disp2 (idexpression of a
> particular type) and disp (identifier) against the first argument of
> snprintf.  This is needed because 1) you want to give a type, which
> requires and idexpression, and 2) in later rules you want to change a
> variable declaration, which requires an identifier.
>
>
> > (
> > T disp[...];
> > |
> > T disp[...]="";
> > )
> > <+...
> >   snprintf(disp, sizeof(disp), prnt);
> > ... when != disp
> > - log_buffer(level, strlen(disp)+1, disp);
> > + log_formatted(level, prnt);
> > ...+>
> > }
> >
> > // Only remove the display variable and snprintf if there are no
> > // other references to the variable.
> >
> > @r2 depends on r1@
> > type r1.T;
> > identifier r1.disp;
> > expression list r1.prnt;
> > @@
> >
> > {
> > ... when any
> > (
> > - T disp[...];
> > |
> > - T disp[...]="";
> > )
> > <+... when != disp
> > - snprintf(disp, sizeof(disp), prnt);
> > ...+>
> > }
>
> Here you could try considering the problem from the opposite point of
> view.

This was the key to getting it work, thanks!

>
> @r2 exists@ // exists is what helps you drop the complexity, by needing to
> type r1.T;  // find only one matching path
> identifier r1.disp;;
> expression list r1.prnt;
> position p;
> @@
>
> (
> T@p disp[...];
> |
> T@p disp[...]="";
> )
> ...
> snprintf(disp, sizeof(disp), prnt);
>
> @r3@
> position p != r2.p;
> type r1.T;
> identifier r1.disp;;
> @@
>
> (
> - T@p disp[...];
> |
> - T@p disp[...]="";
> )
>
>
> Does it matter that the initial value of disp is ""?  In the proposed
> first rule I have dropped that constraint.
>
> julia
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci