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.
>>
>> Signed-off-by: Markus Armbruster <arm...@redhat.com>
>> ---
>> block.c | 1 -
>> block/file-posix.c | 7 +++----
>> block/file-win32.c | 2 +-
>> block/gluster.c | 6 ++----
>> block/parallels.c | 3 ++-
>> block/qcow2.c | 6 ++----
>> blockdev.c | 1 -
>> hmp.c | 2 +-
>> include/qapi/util.h | 2 +-
>> migration/global_state.c | 3 +--
>
> This would be a patch where using scripts/git.orderfile to highlight the
> interface change first would make review a bit quicker :)
>
>> +++ 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.
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?
> Reviewed-by: Eric Blake <ebl...@redhat.com>
Thanks!
