Re: [Qemu-devel] [PATCH v9 16/37] qapi: Swap 'name' in visit_* callbacks to match public API

[Markus Armbruster]

Eric Blake <address@hidden> writes:

> On 01/20/2016 11:55 AM, Markus Armbruster wrote:
>> Eric Blake <address@hidden> writes:
>> 
>>> As explained in the previous patches, matching argument order of
>>> 'name, &value' to JSON's "name":value makes sense.  However,
>>> while the last two patches were easy with Coccinelle, I ended up
>>> doing this one all by hand.  Now all the visitor callbacks match
>>> the main interface.
>>>
>>> Signed-off-by: Eric Blake <address@hidden>
>>> Reviewed-by: Marc-André Lureau <address@hidden>
>>>
>
>>> @@ -30,39 +30,42 @@ struct Visitor
>>>      GenericList *(*next_list)(Visitor *v, GenericList **list, Error 
>>> **errp);
>>>      void (*end_list)(Visitor *v, Error **errp);
>>>
>>> -    void (*type_enum)(Visitor *v, int *obj, const char * const strings[],
>>> -                      const char *kind, const char *name, Error **errp);
>>> +    void (*type_enum)(Visitor *v, const char *name, int *obj,
>>> +                      const char * const strings[], const char *kind,
>> 
>> Opportunity to change to 'const char *const'.  I prefer that, because it
>> makes the fact that this is a pointer-* and not a binary operator-*
>> visually obvious.
>> 
>> Same elsewhere.
>
> Hmm, I probably have churn later in the series.  Will fix.
>
>>>      /* May be NULL; most useful for input visitors. */
>>> -    void (*optional)(Visitor *v, bool *present, const char *name);
>>> +    void (*optional)(Visitor *v, const char *name, bool *present);
>>>
>
>> 
>> I checked the changes to this file carefully.  Can we rely on the
>> compiler to flag mistakes in the rest of the patch?
>
> C's (intentionally-loose) treatment of 'char *' like 'void *' is a bit
> worrisome, but the fact that we have 'const' on only one of the two
> swapped arguments was indeed enough to make the compiler complain about
> mismatch in parameter types when trying to assign incorrectly-typed
> static functions to the updated struct members.

Okay, I guess that's good enough.