On 08/24/2017 01:35 PM, Markus Armbruster wrote: > Eric Blake <ebl...@redhat.com> writes: > >> On 08/24/2017 03:45 AM, Markus Armbruster wrote: >>> The lookup tables have a sentinel, no need to make callers pass their >>> size. >>> >>> Fun: the header has it in the wrong position. Good riddance. >>>
>>> +++ b/include/qapi/util.h
>>> @@ -12,7 +12,7 @@
>>> #define QAPI_UTIL_H
>>>
>>> int qapi_enum_parse(const char * const lookup[], const char *buf,
>>> - int max, int def, Error **errp);
>>> + int def, Error **errp);
>>
>> I'm not sure what you meant by wrong position; were you thinking that
>> lookup/max should be immediately adjacent (since max is a property of
>> the lookup[] parameter), and sticking 'buf' in between the two is what
>> meant 'max' was in the wrong position?
>
> Compare the declaration above with the definition below:
>
> diff --git a/qapi/qapi-util.c b/qapi/qapi-util.c
> index 46eda7d..ee7594f 100644
> --- a/qapi/qapi-util.c
> +++ b/qapi/qapi-util.c
> @@ -16,7 +16,7 @@
> #include "qapi/util.h"
>
> int qapi_enum_parse(const char * const lookup[], const char *buf,
> - int max, int def, Error **errp)
> + int def, Error **errp)
> {
> int i;
>
> Declaration has max before def, definition has it the other way round.
Huh? On current master (commit 248b2373), the two look like they match
to me:
$ git grep -A1 qapi_enum_parse'.*const look'
include/qapi/util.h:int qapi_enum_parse(const char * const lookup[],
const char *buf,
include/qapi/util.h- int max, int def, Error **errp);
--
qapi/qapi-util.c:int qapi_enum_parse(const char * const lookup[], const
char *buf,
qapi/qapi-util.c- int max, int def, Error **errp)
>
> Such errors are one reason I prefer to have documentation next to
> definitions, which are authoritative, rather than declarations, which
> may or may not match the definition.
>
>> The change itself is reasonable, even if the commit message needs a
>> tweak to answer my question.
>
> Care to suggest a wording?
At this point, I find the claim to be bogus, so I suggest you delete the
'Fun:' paragraph.
>
>> Reviewed-by: Eric Blake <ebl...@redhat.com>
>
> Thanks!
>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
