| Message ID | 1503564371-26090-3-git-send-email-armbru@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Thu, Aug 24, 2017 at 10:45 AM, Markus Armbruster <armbru@redhat.com> 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 <armbru@redhat.com> Reviewed-by: Marc-André Lureau <marcandre.lureau@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 +-- > qapi/qapi-util.c | 4 ++-- > qemu-img.c | 3 +-- > qemu-nbd.c | 1 - > tests/test-qapi-util.c | 15 +++++---------- > 14 files changed, 21 insertions(+), 35 deletions(-) > > diff --git a/block.c b/block.c > index 3615a68..dd0efa2 100644 > --- a/block.c > +++ b/block.c > @@ -1335,7 +1335,6 @@ static int bdrv_open_common(BlockDriverState *bs, BlockBackend *file, > BlockdevDetectZeroesOptions value = > qapi_enum_parse(BlockdevDetectZeroesOptions_lookup, > detect_zeroes, > - BLOCKDEV_DETECT_ZEROES_OPTIONS__MAX, > BLOCKDEV_DETECT_ZEROES_OPTIONS_OFF, > &local_err); > if (local_err) { > diff --git a/block/file-posix.c b/block/file-posix.c > index cb3bfce..97e8a92 100644 > --- a/block/file-posix.c > +++ b/block/file-posix.c > @@ -439,7 +439,7 @@ static int raw_open_common(BlockDriverState *bs, QDict *options, > ? BLOCKDEV_AIO_OPTIONS_NATIVE > : BLOCKDEV_AIO_OPTIONS_THREADS; > aio = qapi_enum_parse(BlockdevAioOptions_lookup, qemu_opt_get(opts, "aio"), > - BLOCKDEV_AIO_OPTIONS__MAX, aio_default, &local_err); > + aio_default, &local_err); > if (local_err) { > error_propagate(errp, local_err); > ret = -EINVAL; > @@ -448,7 +448,7 @@ static int raw_open_common(BlockDriverState *bs, QDict *options, > s->use_linux_aio = (aio == BLOCKDEV_AIO_OPTIONS_NATIVE); > > locking = qapi_enum_parse(OnOffAuto_lookup, qemu_opt_get(opts, "locking"), > - ON_OFF_AUTO__MAX, ON_OFF_AUTO_AUTO, &local_err); > + ON_OFF_AUTO_AUTO, &local_err); > if (local_err) { > error_propagate(errp, local_err); > ret = -EINVAL; > @@ -1975,8 +1975,7 @@ static int raw_create(const char *filename, QemuOpts *opts, Error **errp) > nocow = qemu_opt_get_bool(opts, BLOCK_OPT_NOCOW, false); > buf = qemu_opt_get_del(opts, BLOCK_OPT_PREALLOC); > prealloc = qapi_enum_parse(PreallocMode_lookup, buf, > - PREALLOC_MODE__MAX, PREALLOC_MODE_OFF, > - &local_err); > + PREALLOC_MODE_OFF, &local_err); > g_free(buf); > if (local_err) { > error_propagate(errp, local_err); > diff --git a/block/file-win32.c b/block/file-win32.c > index 4706335..978d805 100644 > --- a/block/file-win32.c > +++ b/block/file-win32.c > @@ -304,7 +304,7 @@ static bool get_aio_option(QemuOpts *opts, int flags, Error **errp) > aio_default = (flags & BDRV_O_NATIVE_AIO) ? BLOCKDEV_AIO_OPTIONS_NATIVE > : BLOCKDEV_AIO_OPTIONS_THREADS; > aio = qapi_enum_parse(BlockdevAioOptions_lookup, qemu_opt_get(opts, "aio"), > - BLOCKDEV_AIO_OPTIONS__MAX, aio_default, errp); > + aio_default, errp); > > switch (aio) { > case BLOCKDEV_AIO_OPTIONS_NATIVE: > diff --git a/block/gluster.c b/block/gluster.c > index 3064a45..8367e80 100644 > --- a/block/gluster.c > +++ b/block/gluster.c > @@ -544,8 +544,7 @@ static int qemu_gluster_parse_json(BlockdevOptionsGluster *gconf, > if (!strcmp(ptr, "tcp")) { > ptr = "inet"; /* accept legacy "tcp" */ > } > - type = qapi_enum_parse(SocketAddressType_lookup, ptr, > - SOCKET_ADDRESS_TYPE__MAX, -1, NULL); > + type = qapi_enum_parse(SocketAddressType_lookup, ptr, -1, NULL); > if (type != SOCKET_ADDRESS_TYPE_INET > && type != SOCKET_ADDRESS_TYPE_UNIX) { > error_setg(&local_err, > @@ -1002,8 +1001,7 @@ static int qemu_gluster_create(const char *filename, > BDRV_SECTOR_SIZE); > > tmp = qemu_opt_get_del(opts, BLOCK_OPT_PREALLOC); > - prealloc = qapi_enum_parse(PreallocMode_lookup, tmp, > - PREALLOC_MODE__MAX, PREALLOC_MODE_OFF, > + prealloc = qapi_enum_parse(PreallocMode_lookup, tmp, PREALLOC_MODE_OFF, > &local_err); > g_free(tmp); > if (local_err) { > diff --git a/block/parallels.c b/block/parallels.c > index e1e06d2..eb92366 100644 > --- a/block/parallels.c > +++ b/block/parallels.c > @@ -697,7 +697,8 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags, > s->prealloc_size = MAX(s->tracks, s->prealloc_size >> BDRV_SECTOR_BITS); > buf = qemu_opt_get_del(opts, PARALLELS_OPT_PREALLOC_MODE); > s->prealloc_mode = qapi_enum_parse(prealloc_mode_lookup, buf, > - PRL_PREALLOC_MODE__MAX, PRL_PREALLOC_MODE_FALLOCATE, &local_err); > + PRL_PREALLOC_MODE_FALLOCATE, > + &local_err); > g_free(buf); > if (local_err != NULL) { > goto fail_options; > diff --git a/block/qcow2.c b/block/qcow2.c > index 40ba26c..7aaf248 100644 > --- a/block/qcow2.c > +++ b/block/qcow2.c > @@ -2933,8 +2933,7 @@ static int qcow2_create(const char *filename, QemuOpts *opts, Error **errp) > } > buf = qemu_opt_get_del(opts, BLOCK_OPT_PREALLOC); > prealloc = qapi_enum_parse(PreallocMode_lookup, buf, > - PREALLOC_MODE__MAX, PREALLOC_MODE_OFF, > - &local_err); > + PREALLOC_MODE_OFF, &local_err); > if (local_err) { > error_propagate(errp, local_err); > ret = -EINVAL; > @@ -3624,8 +3623,7 @@ static BlockMeasureInfo *qcow2_measure(QemuOpts *opts, BlockDriverState *in_bs, > > optstr = qemu_opt_get_del(opts, BLOCK_OPT_PREALLOC); > prealloc = qapi_enum_parse(PreallocMode_lookup, optstr, > - PREALLOC_MODE__MAX, PREALLOC_MODE_OFF, > - &local_err); > + PREALLOC_MODE_OFF, &local_err); > g_free(optstr); > if (local_err) { > goto err; > diff --git a/blockdev.c b/blockdev.c > index 02cd69b..722a61e 100644 > --- a/blockdev.c > +++ b/blockdev.c > @@ -440,7 +440,6 @@ static void extract_common_blockdev_options(QemuOpts *opts, int *bdrv_flags, > *detect_zeroes = > qapi_enum_parse(BlockdevDetectZeroesOptions_lookup, > qemu_opt_get(opts, "detect-zeroes"), > - BLOCKDEV_DETECT_ZEROES_OPTIONS__MAX, > BLOCKDEV_DETECT_ZEROES_OPTIONS_OFF, > &local_error); > if (local_error) { > diff --git a/hmp.c b/hmp.c > index fd80dce..03c1a78 100644 > --- a/hmp.c > +++ b/hmp.c > @@ -1742,7 +1742,7 @@ void hmp_change(Monitor *mon, const QDict *qdict) > if (read_only) { > read_only_mode = > qapi_enum_parse(BlockdevChangeReadOnlyMode_lookup, > - read_only, BLOCKDEV_CHANGE_READ_ONLY_MODE__MAX, > + read_only, > BLOCKDEV_CHANGE_READ_ONLY_MODE_RETAIN, &err); > if (err) { > hmp_handle_error(mon, &err); > diff --git a/include/qapi/util.h b/include/qapi/util.h > index 7436ed8..4eb8a3f 100644 > --- a/include/qapi/util.h > +++ 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); > > int parse_qapi_name(const char *name, bool complete); > > diff --git a/migration/global_state.c b/migration/global_state.c > index dcbbcb2..88c55f8 100644 > --- a/migration/global_state.c > +++ b/migration/global_state.c > @@ -89,8 +89,7 @@ static int global_state_post_load(void *opaque, int version_id) > s->received = true; > trace_migrate_global_state_post_load(runstate); > > - r = qapi_enum_parse(RunState_lookup, runstate, RUN_STATE__MAX, > - -1, &local_err); > + r = qapi_enum_parse(RunState_lookup, runstate, -1, &local_err); > > if (r == -1) { > if (local_err) { > 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; > > @@ -24,7 +24,7 @@ int qapi_enum_parse(const char * const lookup[], const char *buf, > return def; > } > > - for (i = 0; i < max; i++) { > + for (i = 0; lookup[i]; i++) { > if (!strcmp(buf, lookup[i])) { > return i; > } > diff --git a/qemu-img.c b/qemu-img.c > index 56ef49e..611ab7d 100644 > --- a/qemu-img.c > +++ b/qemu-img.c > @@ -3491,8 +3491,7 @@ static int img_resize(int argc, char **argv) > break; > case OPTION_PREALLOCATION: > prealloc = qapi_enum_parse(PreallocMode_lookup, optarg, > - PREALLOC_MODE__MAX, PREALLOC_MODE__MAX, > - NULL); > + PREALLOC_MODE__MAX, NULL); > if (prealloc == PREALLOC_MODE__MAX) { > error_report("Invalid preallocation mode '%s'", optarg); > return 1; > diff --git a/qemu-nbd.c b/qemu-nbd.c > index 27164b8..96e10d6 100644 > --- a/qemu-nbd.c > +++ b/qemu-nbd.c > @@ -641,7 +641,6 @@ int main(int argc, char **argv) > detect_zeroes = > qapi_enum_parse(BlockdevDetectZeroesOptions_lookup, > optarg, > - BLOCKDEV_DETECT_ZEROES_OPTIONS__MAX, > BLOCKDEV_DETECT_ZEROES_OPTIONS_OFF, > &local_err); > if (local_err) { > diff --git a/tests/test-qapi-util.c b/tests/test-qapi-util.c > index e869757..d72ee8c 100644 > --- a/tests/test-qapi-util.c > +++ b/tests/test-qapi-util.c > @@ -20,25 +20,20 @@ static void test_qapi_enum_parse(void) > Error *err = NULL; > int ret; > > - ret = qapi_enum_parse(QType_lookup, NULL, QTYPE__MAX, QTYPE_NONE, > - &error_abort); > + ret = qapi_enum_parse(QType_lookup, NULL, QTYPE_NONE, &error_abort); > g_assert_cmpint(ret, ==, QTYPE_NONE); > > - ret = qapi_enum_parse(QType_lookup, "junk", QTYPE__MAX, -1, > - NULL); > + ret = qapi_enum_parse(QType_lookup, "junk", -1, NULL); > g_assert_cmpint(ret, ==, -1); > > - ret = qapi_enum_parse(QType_lookup, "junk", QTYPE__MAX, -1, > - &err); > + ret = qapi_enum_parse(QType_lookup, "junk", -1, &err); > error_free_or_abort(&err); > > - ret = qapi_enum_parse(QType_lookup, "none", QTYPE__MAX, -1, > - &error_abort); > + ret = qapi_enum_parse(QType_lookup, "none", -1, &error_abort); > g_assert_cmpint(ret, ==, QTYPE_NONE); > > ret = qapi_enum_parse(QType_lookup, QType_lookup[QTYPE__MAX - 1], > - QTYPE__MAX, QTYPE__MAX - 1, > - &error_abort); > + QTYPE__MAX - 1, &error_abort); > g_assert_cmpint(ret, ==, QTYPE__MAX - 1); > } > > -- > 2.7.5 > >
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 <armbru@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? The change itself is reasonable, even if the commit message needs a tweak to answer my question. Reviewed-by: Eric Blake <eblake@redhat.com>
Eric Blake <eblake@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 <armbru@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 <eblake@redhat.com> Thanks!
On 08/24/2017 01:35 PM, Markus Armbruster wrote: > Eric Blake <eblake@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 <eblake@redhat.com> > > Thanks! >
Eric Blake <eblake@redhat.com> writes: > On 08/24/2017 01:35 PM, Markus Armbruster wrote: >> Eric Blake <eblake@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. Sure. >>> Reviewed-by: Eric Blake <eblake@redhat.com> >> >> Thanks!
diff --git a/block.c b/block.c index 3615a68..dd0efa2 100644 --- a/block.c +++ b/block.c @@ -1335,7 +1335,6 @@ static int bdrv_open_common(BlockDriverState *bs, BlockBackend *file, BlockdevDetectZeroesOptions value = qapi_enum_parse(BlockdevDetectZeroesOptions_lookup, detect_zeroes, - BLOCKDEV_DETECT_ZEROES_OPTIONS__MAX, BLOCKDEV_DETECT_ZEROES_OPTIONS_OFF, &local_err); if (local_err) { diff --git a/block/file-posix.c b/block/file-posix.c index cb3bfce..97e8a92 100644 --- a/block/file-posix.c +++ b/block/file-posix.c @@ -439,7 +439,7 @@ static int raw_open_common(BlockDriverState *bs, QDict *options, ? BLOCKDEV_AIO_OPTIONS_NATIVE : BLOCKDEV_AIO_OPTIONS_THREADS; aio = qapi_enum_parse(BlockdevAioOptions_lookup, qemu_opt_get(opts, "aio"), - BLOCKDEV_AIO_OPTIONS__MAX, aio_default, &local_err); + aio_default, &local_err); if (local_err) { error_propagate(errp, local_err); ret = -EINVAL; @@ -448,7 +448,7 @@ static int raw_open_common(BlockDriverState *bs, QDict *options, s->use_linux_aio = (aio == BLOCKDEV_AIO_OPTIONS_NATIVE); locking = qapi_enum_parse(OnOffAuto_lookup, qemu_opt_get(opts, "locking"), - ON_OFF_AUTO__MAX, ON_OFF_AUTO_AUTO, &local_err); + ON_OFF_AUTO_AUTO, &local_err); if (local_err) { error_propagate(errp, local_err); ret = -EINVAL; @@ -1975,8 +1975,7 @@ static int raw_create(const char *filename, QemuOpts *opts, Error **errp) nocow = qemu_opt_get_bool(opts, BLOCK_OPT_NOCOW, false); buf = qemu_opt_get_del(opts, BLOCK_OPT_PREALLOC); prealloc = qapi_enum_parse(PreallocMode_lookup, buf, - PREALLOC_MODE__MAX, PREALLOC_MODE_OFF, - &local_err); + PREALLOC_MODE_OFF, &local_err); g_free(buf); if (local_err) { error_propagate(errp, local_err); diff --git a/block/file-win32.c b/block/file-win32.c index 4706335..978d805 100644 --- a/block/file-win32.c +++ b/block/file-win32.c @@ -304,7 +304,7 @@ static bool get_aio_option(QemuOpts *opts, int flags, Error **errp) aio_default = (flags & BDRV_O_NATIVE_AIO) ? BLOCKDEV_AIO_OPTIONS_NATIVE : BLOCKDEV_AIO_OPTIONS_THREADS; aio = qapi_enum_parse(BlockdevAioOptions_lookup, qemu_opt_get(opts, "aio"), - BLOCKDEV_AIO_OPTIONS__MAX, aio_default, errp); + aio_default, errp); switch (aio) { case BLOCKDEV_AIO_OPTIONS_NATIVE: diff --git a/block/gluster.c b/block/gluster.c index 3064a45..8367e80 100644 --- a/block/gluster.c +++ b/block/gluster.c @@ -544,8 +544,7 @@ static int qemu_gluster_parse_json(BlockdevOptionsGluster *gconf, if (!strcmp(ptr, "tcp")) { ptr = "inet"; /* accept legacy "tcp" */ } - type = qapi_enum_parse(SocketAddressType_lookup, ptr, - SOCKET_ADDRESS_TYPE__MAX, -1, NULL); + type = qapi_enum_parse(SocketAddressType_lookup, ptr, -1, NULL); if (type != SOCKET_ADDRESS_TYPE_INET && type != SOCKET_ADDRESS_TYPE_UNIX) { error_setg(&local_err, @@ -1002,8 +1001,7 @@ static int qemu_gluster_create(const char *filename, BDRV_SECTOR_SIZE); tmp = qemu_opt_get_del(opts, BLOCK_OPT_PREALLOC); - prealloc = qapi_enum_parse(PreallocMode_lookup, tmp, - PREALLOC_MODE__MAX, PREALLOC_MODE_OFF, + prealloc = qapi_enum_parse(PreallocMode_lookup, tmp, PREALLOC_MODE_OFF, &local_err); g_free(tmp); if (local_err) { diff --git a/block/parallels.c b/block/parallels.c index e1e06d2..eb92366 100644 --- a/block/parallels.c +++ b/block/parallels.c @@ -697,7 +697,8 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags, s->prealloc_size = MAX(s->tracks, s->prealloc_size >> BDRV_SECTOR_BITS); buf = qemu_opt_get_del(opts, PARALLELS_OPT_PREALLOC_MODE); s->prealloc_mode = qapi_enum_parse(prealloc_mode_lookup, buf, - PRL_PREALLOC_MODE__MAX, PRL_PREALLOC_MODE_FALLOCATE, &local_err); + PRL_PREALLOC_MODE_FALLOCATE, + &local_err); g_free(buf); if (local_err != NULL) { goto fail_options; diff --git a/block/qcow2.c b/block/qcow2.c index 40ba26c..7aaf248 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -2933,8 +2933,7 @@ static int qcow2_create(const char *filename, QemuOpts *opts, Error **errp) } buf = qemu_opt_get_del(opts, BLOCK_OPT_PREALLOC); prealloc = qapi_enum_parse(PreallocMode_lookup, buf, - PREALLOC_MODE__MAX, PREALLOC_MODE_OFF, - &local_err); + PREALLOC_MODE_OFF, &local_err); if (local_err) { error_propagate(errp, local_err); ret = -EINVAL; @@ -3624,8 +3623,7 @@ static BlockMeasureInfo *qcow2_measure(QemuOpts *opts, BlockDriverState *in_bs, optstr = qemu_opt_get_del(opts, BLOCK_OPT_PREALLOC); prealloc = qapi_enum_parse(PreallocMode_lookup, optstr, - PREALLOC_MODE__MAX, PREALLOC_MODE_OFF, - &local_err); + PREALLOC_MODE_OFF, &local_err); g_free(optstr); if (local_err) { goto err; diff --git a/blockdev.c b/blockdev.c index 02cd69b..722a61e 100644 --- a/blockdev.c +++ b/blockdev.c @@ -440,7 +440,6 @@ static void extract_common_blockdev_options(QemuOpts *opts, int *bdrv_flags, *detect_zeroes = qapi_enum_parse(BlockdevDetectZeroesOptions_lookup, qemu_opt_get(opts, "detect-zeroes"), - BLOCKDEV_DETECT_ZEROES_OPTIONS__MAX, BLOCKDEV_DETECT_ZEROES_OPTIONS_OFF, &local_error); if (local_error) { diff --git a/hmp.c b/hmp.c index fd80dce..03c1a78 100644 --- a/hmp.c +++ b/hmp.c @@ -1742,7 +1742,7 @@ void hmp_change(Monitor *mon, const QDict *qdict) if (read_only) { read_only_mode = qapi_enum_parse(BlockdevChangeReadOnlyMode_lookup, - read_only, BLOCKDEV_CHANGE_READ_ONLY_MODE__MAX, + read_only, BLOCKDEV_CHANGE_READ_ONLY_MODE_RETAIN, &err); if (err) { hmp_handle_error(mon, &err); diff --git a/include/qapi/util.h b/include/qapi/util.h index 7436ed8..4eb8a3f 100644 --- a/include/qapi/util.h +++ 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); int parse_qapi_name(const char *name, bool complete); diff --git a/migration/global_state.c b/migration/global_state.c index dcbbcb2..88c55f8 100644 --- a/migration/global_state.c +++ b/migration/global_state.c @@ -89,8 +89,7 @@ static int global_state_post_load(void *opaque, int version_id) s->received = true; trace_migrate_global_state_post_load(runstate); - r = qapi_enum_parse(RunState_lookup, runstate, RUN_STATE__MAX, - -1, &local_err); + r = qapi_enum_parse(RunState_lookup, runstate, -1, &local_err); if (r == -1) { if (local_err) { 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; @@ -24,7 +24,7 @@ int qapi_enum_parse(const char * const lookup[], const char *buf, return def; } - for (i = 0; i < max; i++) { + for (i = 0; lookup[i]; i++) { if (!strcmp(buf, lookup[i])) { return i; } diff --git a/qemu-img.c b/qemu-img.c index 56ef49e..611ab7d 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -3491,8 +3491,7 @@ static int img_resize(int argc, char **argv) break; case OPTION_PREALLOCATION: prealloc = qapi_enum_parse(PreallocMode_lookup, optarg, - PREALLOC_MODE__MAX, PREALLOC_MODE__MAX, - NULL); + PREALLOC_MODE__MAX, NULL); if (prealloc == PREALLOC_MODE__MAX) { error_report("Invalid preallocation mode '%s'", optarg); return 1; diff --git a/qemu-nbd.c b/qemu-nbd.c index 27164b8..96e10d6 100644 --- a/qemu-nbd.c +++ b/qemu-nbd.c @@ -641,7 +641,6 @@ int main(int argc, char **argv) detect_zeroes = qapi_enum_parse(BlockdevDetectZeroesOptions_lookup, optarg, - BLOCKDEV_DETECT_ZEROES_OPTIONS__MAX, BLOCKDEV_DETECT_ZEROES_OPTIONS_OFF, &local_err); if (local_err) { diff --git a/tests/test-qapi-util.c b/tests/test-qapi-util.c index e869757..d72ee8c 100644 --- a/tests/test-qapi-util.c +++ b/tests/test-qapi-util.c @@ -20,25 +20,20 @@ static void test_qapi_enum_parse(void) Error *err = NULL; int ret; - ret = qapi_enum_parse(QType_lookup, NULL, QTYPE__MAX, QTYPE_NONE, - &error_abort); + ret = qapi_enum_parse(QType_lookup, NULL, QTYPE_NONE, &error_abort); g_assert_cmpint(ret, ==, QTYPE_NONE); - ret = qapi_enum_parse(QType_lookup, "junk", QTYPE__MAX, -1, - NULL); + ret = qapi_enum_parse(QType_lookup, "junk", -1, NULL); g_assert_cmpint(ret, ==, -1); - ret = qapi_enum_parse(QType_lookup, "junk", QTYPE__MAX, -1, - &err); + ret = qapi_enum_parse(QType_lookup, "junk", -1, &err); error_free_or_abort(&err); - ret = qapi_enum_parse(QType_lookup, "none", QTYPE__MAX, -1, - &error_abort); + ret = qapi_enum_parse(QType_lookup, "none", -1, &error_abort); g_assert_cmpint(ret, ==, QTYPE_NONE); ret = qapi_enum_parse(QType_lookup, QType_lookup[QTYPE__MAX - 1], - QTYPE__MAX, QTYPE__MAX - 1, - &error_abort); + QTYPE__MAX - 1, &error_abort); g_assert_cmpint(ret, ==, QTYPE__MAX - 1); }
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 <armbru@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 +-- qapi/qapi-util.c | 4 ++-- qemu-img.c | 3 +-- qemu-nbd.c | 1 - tests/test-qapi-util.c | 15 +++++---------- 14 files changed, 21 insertions(+), 35 deletions(-)