[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v9 15/37] qom: Swap 'name' next to visitor in Ob

From: Eric Blake
Subject: Re: [Qemu-devel] [PATCH v9 15/37] qom: Swap 'name' next to visitor in ObjectPropertyAccessor
Date: Wed, 20 Jan 2016 13:54:41 -0700
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.0

On 01/20/2016 11:49 AM, Markus Armbruster wrote:
> Eric Blake <address@hidden> writes:
>> Similar to the previous patch, it's nice to have all functions
>> in the tree that involve a visitor and a name for conversion to
>> or from QAPI to consistently stick the 'name' parameter next
>> to the Visitor parameter.
>> Done by manually changing include/qom/object.h and qom/object.c,
>> then running this Coccinelle script and touching up the fallout
>> (Coccinelle insisted on adding some trailing whitespace).
>>     @ rule1 @
>>     identifier fn;
>>     type Object, Visitor, Error;
>>     identifier obj, v, opaque, name, errp;
>>     @@
>>      void fn
>>     - (Object *obj, Visitor *v, void *opaque, const char *name,
>>     + (Object *obj, Visitor *v, const char *name, void *opaque,
>>        Error **errp) { ... }
> I think we want to match void functions with exactly these parameter
> types.  The parameter names don't matter.

The parameter names shouldn't matter; the 'identifier obj' should have
been enough to make 'obj' a metavariable matching any actual parameter name.

> However, the actual match is looser!  For instance, it also matches
>     void foo(int *pi, unsigned *pu, void *vp, const char *cp, double **dpp)
>     {
>     }

Uggh. My intent was to match exactly 'Object *' and 'Visitor *' as the
first two types, where 'int *' and 'unsigned *' are NOT matches.  But I
don't know Coccinelle well enough to make that blatantly obvious (is my
declaration of 'type Object' not correct?).

> This could mess up unrelated function.  I could double-check it doesn't,
> but I'd rather have a narrower match instead.  Can't give one offhand,
> though.  Ideas?

Is 'typedef' better than 'type' for constraining the type of the first
two arguments?  Or does Coccinelle do literal matches on anything you
don't pre-declare, as in:

     @ rule1 @
     identifier fn;
     identifier obj, v, opaque, name, errp;
      void fn
     - (Object *obj, Visitor *v, void *opaque, const char *name,
     + (Object *obj, Visitor *v, const char *name, void *opaque,
        Error **errp) { ... }

Fortunately, a manual inspection of the results (which I had to do
anyways due to spacing issues) didn't spot any incorrect swaps.

At this point, I don't know that re-writing Coccinelle will be worth the
hassle (nothing else needs to be rewritten).

>>     @@
>>     identifier rule1.fn;
>>     expression obj, v, opaque, name, errp;
>>     @@
>>      fn(obj, v,
>>     -   opaque, name,
>>     +   name, opaque,
>>         errp)
> The rule1.fn restricts the match to functions changed by the previous
> rule.  Good.
>> Signed-off-by: Eric Blake <address@hidden>
>> Reviewed-by: Marc-André Lureau <address@hidden>

Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

reply via email to

[Prev in Thread] Current Thread [Next in Thread]