[Top][All Lists]

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

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

From: Markus Armbruster
Subject: Re: [Qemu-devel] [PATCH v9 16/37] qapi: Swap 'name' in visit_* callbacks to match public API
Date: Thu, 21 Jan 2016 10:19:02 +0100
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux)

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.

reply via email to

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