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

[Markus Armbruster]

Kevin Wolf <address@hidden> writes:

> Am 21.01.2016 um 10:18 hat Markus Armbruster geschrieben:
>> Eric Blake <address@hidden> writes:
>> > On 01/20/2016 11:49 AM, Markus Armbruster wrote:
>> >> Eric Blake <address@hidden> writes:
>> >> 
>> >> 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?).
>> 
>> 'type Object' makes 'Object' a metavariable matching any C type.
>
> I can't say anything on this one, because I've never used 'type'. You
> may or may not be right. However...
>
>> >> 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?
>> 
>> Matches any C typedef name.  Less wrong, but still wrong :)
>
> ...I'm pretty sure that this is wrong and 'typedef' only declares that a
> specific type exists as a typedef.

Coccinelle's documentation is awfully terse.  Looks like I jumped to
conclusions.

>> >                 Or does Coccinelle do literal matches on anything you
>> > don't pre-declare, as in:
>> 
>> Yes, but...
>> 
>> >      @ 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) { ... }
>> 
>> ... when I try that, spatch throws a parse error.
>
> Because you need to declare the typedef. :-)
>
> I had a similar problem and asked Julia about it at KVM Forum, so I'm
> pretty sure that it's right.

If I replace 'type' by 'typedef', spatch no longer messes up

    void foo(int *pi, unsigned *pu, void *vp, const char *cp, double **dpp)

but still rewrites

    void bar(Object *o, Visitor *v, void *opq, const char *n, Error **ep)

Thanks, Kevin!

Eric, could you try to regenerate your patch with the corrected script?