On Fri, 2011-02-18 at 10:34 +0100, ext Nicolas Palix wrote:
> Hi,
> 
> The problem with our example is that your mixing GetScreenSizeRange
> with SetScreenConfig. Is it normal ?

No, that was accidental, as I mentioned in the email. See further down
for details.

> Moreover, you need to handle the cast. A possibility is:
> 
> @@
> type T;
> type C;
> identifier foobar;
> identifier client;
> @@
> -T foobar;
> +REPLY(T);
> ...
> (
> -WriteToClient(client, sizeof(T), (C)&foobar);
> +WriteToClient(client, sizeof(T), (C)&rep);
> |
> -WriteToClient(client, sizeof(T), &foobar);
> +WriteToClient(client, sizeof(T), &rep);
> )
> 
> In that case, the cast is preserved if it exists. You can change the
> last "+" line
> to always add a cast or change the previous "+" line to always remove
> it.

Ok, good to know about the cast handling... but it still doesn't seem to
work on the test.c code (with the mixed-up type fixed.)

If it's working for you, perhaps I have an older version, or a version
with a bug? This it the only output that I get:

$ spatch -sp_file test.cocci test.c
init_defs_builtins: /usr/share/coccinelle/standard.h
HANDLING: test.c

No patch is generated, test.c is not modified. I'm a bit confused as to
why it doesn't work.

> 
> On Fri, Feb 18, 2011 at 10:05 AM, Oliver McFadden
> <[email protected]> wrote:
>         Oh, please note, there is one bug in test.c. The struct
>         typedef should
>         of course be named xRRSetScreenConfigReply to match to
>         WriteToClient
>         call.
>         
>         However fixing this unfortunately does not make the SmPL patch
>         work.
>         
>         -- Oliver.
>         
>         
>         On Fri, 2011-02-18 at 11:03 +0200, Oliver McFadden wrote:
>         > On Fri, 2011-02-18 at 09:03 +0100, ext Nicolas Palix wrote:
>         > > Hi,
>         > >
>         > > On Fri, Feb 18, 2011 at 4:04 AM, Oliver McFadden
>         > > <[email protected]> wrote:
>         > >         Hi Nicolas,
>         > >
>         > >         I think your solution is closest; I really should
>         learn the
>         > >         SmPL grammar
>         > >         properly... Unfortunately it doesn't seem to work.
>         I did
>         > >         receive a
>         > >         warning:
>         > >
>         > >         warning: line 8: should client be a metavariable?
>         > >
>         > >         I guess in this case it doesn't matter because the
>         variable in
>         > >         the
>         > >         example is actually named "client", but this might
>         not always
>         > >         be the
>         > >         case.
>         > >
>         > > Indeed. I need the client declaration to give you the
>         proper client
>         > > metavariable declaration
>         > > if you care about typing, otherwise you just need to add
>         "identifier
>         > > client" to the set of metavariable.
>         >
>         > Well, I could look that up from the headers. It should just
>         be a
>         > standard C structure.
>         >
>         > However, the problem is your SmPL code below does not work.
>         You can try
>         > it on the example code I've attached. I'd appreciate any
>         help in getting
>         > it working correctly. :-)
>         >
>         > >
>         > >
>         > >
>         > >         Here's a real-world code segment from the X
>         server:
>         > >
>         > >         int main(void)
>         > >         {
>         > >            xRRGetScreenSizeRangeReply  rep;
>         > >            /* normally lots of code here */
>         > >            WriteToClient(client,
>         sizeof(xRRSetScreenConfigReply),
>         > >         (char *)&rep);
>         > >         }
>         > >
>         > >         So I would want to match on the WriteToClient
>         line, see that
>         > >         "rep" (could be named anything) is passed to
>         WriteToClient,
>         > >         then change
>         > >         it's variable definition to a macro:
>         > >
>         > > The fact that "rep" could be named otherwise is already
>         handle by the
>         > > "foobar" metavariable
>         > > of my previous SmPL example.
>         >
>         > Yes.
>         >
>         > >
>         > >
>         > >
>         > >         REPLY(xRRSetScreenConfigReply);
>         > >         /* normally lots of code here */
>         > >         WriteToClient(client,
>         sizeof(xRRSetScreenConfigReply), (char
>         > >         *)&rep);
>         > >
>         > >         Btw, thanks for the help!
>         > >
>         > > Am I correct in assuming that "rep" is now a constant ?
>         >
>         > Yes, I intend to have the REPLY macro always expand to a
>         variable named
>         > "rep" (or "reply" -- I can easily change that myself in the
>         SmPL code.)
>         >
>         > >
>         > >
>         
> --8<--------------------------------------------------->8----------------------------------
>         > >
>         > > @@
>         > > type T;
>         > > identifier foobar;
>         > > identifier client;
>         > > @@
>         > > -T foobar;
>         > > +REPLY(T);
>         > > ...
>         > > -WriteToClient(client, sizeof(T), &foobar);
>         > > +WriteToClient(client, sizeof(T), &rep);
>         > >
>         > >
>         
> --8<--------------------------------------------------->8----------------------------------
>         > >
>         > >
>         > >
>         > >         -- Oliver.
>         > >
>         > >
>         > >         On Wed, 2011-02-16 at 13:27 +0100, ext Nicolas
>         Palix wrote:
>         > >         > Hi,
>         > >         >
>         > >         > Does the following SmPL code do what you want ?
>         > >         > Or did I miss something in your explanation ?
>         > >         >
>         > >         > @@
>         > >         > type T;
>         > >         > identifier foobar;
>         > >         > @@
>         > >         > -T foobar;
>         > >         > +REPLY(T);
>         > >         > ...
>         > >         > -WriteToClient(client, sizeof(T), &foobar);
>         > >         > +WriteToClient(client, sizeof(T), &rep);
>         > >         >
>         > >         > The type and identifier are meta-variables
>         > >         > but "rep" is constant, as it is defined in your
>         macro.
>         > >         >
>         > >         >
>         > >         > On Wed, Feb 16, 2011 at 11:32 AM, Julia Lawall
>         > >         <[email protected]> wrote:
>         > >         >         On Wed, 16 Feb 2011, Oliver McFadden
>         wrote:
>         > >         >
>         > >         >         > Hi,
>         > >         >         >
>         > >         >         > I'm quite new to Coccinelle but it
>         seems to be an
>         > >         excellent
>         > >         >         tool,
>         > >         >         > however, I have a question about how
>         to use it to
>         > >         do what I
>         > >         >         need.
>         > >         >         >
>         > >         >         > I have a bunch of code that looks like
>         this
>         > >         (inside a
>         > >         >         function):
>         > >         >         >
>         > >         >         > xFoobarReply foobar;
>         > >         >         > /* many lines of code */
>         > >         >         > WriteToClient(client,
>         sizeof(xFoobarReply),
>         > >         &foobar);
>         > >         >         >
>         > >         >         > I would like to transform these into
>         (note change
>         > >         of foobar
>         > >         >         to rep):
>         > >         >         >
>         > >         >         > REPLY(xFoobarReply);
>         > >         >         > /* many lines of code */
>         > >         >         > WriteToClient(client,
>         sizeof(xFoobarReply), &rep);
>         > >         >         >
>         > >         >         > Then the REPLY macro is defined to
>         expand to:
>         > >         >         > xFoobarReply rep; memset(&rep, 0,
>         > >         sizeof(xFoobarReply);
>         > >         >         >
>         > >         >         > But note that I must only match on
>         calls to
>         > >         WriteToClient. I
>         > >         >         know how I
>         > >         >         > could change the WriteToClient line
>         easily, but
>         > >         not how to
>         > >         >         change the
>         > >         >         > declaration of one of it's arguments?
>         > >         >
>         > >         >
>         > >         >         Thank you for your interest in
>         Coccinelle.
>         > >         >
>         > >         >         When you say "change the declaration of
>         one of it's
>         > >         >         arguments", do you
>         > >         >         actually mean change the parameter
>         declaration of
>         > >         the
>         > >         >         definition of the
>         > >         >         WriteToClient function?
>         > >         >
>         > >         >         If so, then perhaps the following page
>         in the wiki
>         > >         can help
>         > >         >         you:
>         > >         >
>         > >         >
>         > >
>         
> http://cocci.ekstranet.diku.dk/wiki/doku.php?id=parameter_list_matching
>         > >         >
>         > >         >         This is going the other way, from when
>         you have
>         > >         information
>         > >         >         about a
>         > >         >         parameter argument position and want to
>         adjust the
>         > >         >         corresponding call
>         > >         >         sites, but it should be adaptable to
>         what I think
>         > >         you want to
>         > >         >         do.
>         > >         >
>         > >         >         If I have completely missed
>         understanding, please
>         > >         write back
>         > >         >         and I or
>         > >         >         someone else will get back to you later.
>         > >         >
>         > >         >         julia
>         > >         >
>         > >         >
>         _______________________________________________
>         > >         >         Cocci mailing list
>         > >         >         [email protected]
>         > >         >
>         http://lists.diku.dk/mailman/listinfo/cocci
>         > >         >         (Web access from inside DIKUs LAN only)
>         > >         >
>         > >         >
>         > >         >
>         > >         >
>         > >         > --
>         > >         > Nicolas Palix
>         > >         > http://sardes.inrialpes.fr/~npalix/
>         > >
>         > >
>         > >
>         > >
>         > >
>         > >
>         > > --
>         > > Nicolas Palix
>         > > http://sardes.inrialpes.fr/~npalix/
>         >
>         
>         
>         _______________________________________________
>         Cocci mailing list
>         [email protected]
>         http://lists.diku.dk/mailman/listinfo/cocci
>         (Web access from inside DIKUs LAN only)
>         
> 
> 
> 
> -- 
> Nicolas Palix
> http://sardes.inrialpes.fr/~npalix/


_______________________________________________
Cocci mailing list
[email protected]
http://lists.diku.dk/mailman/listinfo/cocci
(Web access from inside DIKUs LAN only)

Reply via email to