[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.
[Qemu-devel] [PATCH v9 35/37] qapi: Change visit_type_FOO() to no longer return partial objects, Eric Blake, 2016/01/19