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

[Eric Blake]

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.

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

signature.asc
Description: OpenPGP digital signature