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

[George McCollister <george.mccollister@gmail.com>]

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