[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 02/16] qapi: Drop superfluous qapi_enum_parse()
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH 02/16] qapi: Drop superfluous qapi_enum_parse() parameter max |
Date: |
Thu, 24 Aug 2017 20:35:04 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/25.2 (gnu/linux) |
Eric Blake <address@hidden> 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 <address@hidden>
>> ---
>> 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 <address@hidden>
Thanks!
- Re: [Qemu-devel] [PATCH 04/16] tpm: Clean up model registration & lookup, (continued)
[Qemu-devel] [PATCH 07/16] block: Use qemu_enum_parse() in blkdebug_debug_breakpoint(), Markus Armbruster, 2017/08/24
[Qemu-devel] [PATCH 03/16] tpm: Clean up driver registration & lookup, Markus Armbruster, 2017/08/24
[Qemu-devel] [PATCH 09/16] crypto: Use qapi_enum_parse() in qcrypto_block_luks_name_lookup(), Markus Armbruster, 2017/08/24
[Qemu-devel] [PATCH 15/16] qapi: Change data type of the FOO_lookup generated for enum FOO, Markus Armbruster, 2017/08/24
[Qemu-devel] [PATCH 06/16] hmp: Use qapi_enum_parse() in hmp_migrate_set_parameter(), Markus Armbruster, 2017/08/24