[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 06/54] qapi: introduce qapi_enum_lookup()
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH v2 06/54] qapi: introduce qapi_enum_lookup() |
Date: |
Wed, 23 Aug 2017 10:02:12 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/25.2 (gnu/linux) |
Marc-André Lureau <address@hidden> writes:
> This will help with the introduction of a new structure to handle
> enum lookup.
>
> Signed-off-by: Marc-André Lureau <address@hidden>
> ---
[...]
> 45 files changed, 206 insertions(+), 122 deletions(-)
Hmm.
> diff --git a/include/qapi/util.h b/include/qapi/util.h
> index 7436ed815c..60733b6a80 100644
> --- a/include/qapi/util.h
> +++ b/include/qapi/util.h
> @@ -11,6 +11,8 @@
> #ifndef QAPI_UTIL_H
> #define QAPI_UTIL_H
>
> +const char *qapi_enum_lookup(const char * const lookup[], int val);
> +
> int qapi_enum_parse(const char * const lookup[], const char *buf,
> int max, int def, Error **errp);
>
> diff --git a/backends/hostmem.c b/backends/hostmem.c
> index 4606b73849..c4f795475c 100644
> --- a/backends/hostmem.c
> +++ b/backends/hostmem.c
> @@ -13,6 +13,7 @@
> #include "sysemu/hostmem.h"
> #include "hw/boards.h"
> #include "qapi/error.h"
> +#include "qapi/util.h"
> #include "qapi/visitor.h"
> #include "qapi-types.h"
> #include "qapi-visit.h"
> @@ -304,7 +305,7 @@ host_memory_backend_memory_complete(UserCreatable *uc,
> Error **errp)
> return;
> } else if (maxnode == 0 && backend->policy != MPOL_DEFAULT) {
> error_setg(errp, "host-nodes must be set for policy %s",
> - HostMemPolicy_lookup[backend->policy]);
> + qapi_enum_lookup(HostMemPolicy_lookup, backend->policy));
> return;
> }
>
Lookup becomes even more verbose.
Could we claw back some readability with macros? Say in addition to
typedef enum FOO {
...
} FOO;
extern const char *const FOO_lookup[];
generate
#define FOO_str(val) qapi_enum_lookup(FOO_lookup, (val))
Needs a matching qapi-code-gen.txt update.
With that, this patch hunk would become
error_setg(errp, "host-nodes must be set for policy %s",
- HostMemPolicy_lookup[backend->policy]);
+ HostMemPolicy_str(backend->policy);
Perhaps we could even throw in some type checking:
#define FOO_str(val) (type_check(typeof((val)), FOO) \
+ qapi_enum_lookup(FOO_lookup, (val)))
What do you think? Want me to explore a fixup patch you could squash
in?
[Skipping lots of mechanical changes...]
I think you missed test-qobject-input-visitor.c and
string-input-visitor.c.
In test-qobject-input-visitor.c's test_visitor_in_enum():
v = visitor_input_test_init(data, "%s", EnumOne_lookup[i]);
You update it in PATCH 12, but the code only works as long as EnumOne
has no holes. Mapping the enum to string like we do everywhere else in
this patch would be cleaner.
The loop control also subscripts EnumOne_lookup[i]. You take care of
that one in PATCH 12. That's okay.
Same for test-string-input-visitor.c's test_visitor_in_enum().
There's one more in test_native_list_integer_helper():
g_string_append_printf(gstr_union, "{ 'type': '%s', 'data': [ %s ] }",
UserDefNativeListUnionKind_lookup[kind],
gstr_list->str);
Same story.
The patch doesn't touch the _lookup[] subscripts you're going to replace
by qapi_enum_parse() in PATCH 07-11. Understand, but I'd reorder the
patches: first replace by qapi_enum_parse(), because DRY (no need to
explain more at that point). Then get rid of all the remaining
subscripting into _lookup[], i.e. this patch (explain it helps the next
one), then PATCH 12.
- Re: [Qemu-devel] [PATCH v2 51/54] qapi: make s390 commands depend on TARGET_S390X, (continued)
- [Qemu-devel] [PATCH v2 25/54] qapi-visit: add #if conditions to visitors, Marc-André Lureau, 2017/08/22
- [Qemu-devel] [PATCH v2 48/54] tests/qmp-test: add query-qmp-schema test, Marc-André Lureau, 2017/08/22
- [Qemu-devel] [PATCH v2 09/54] hmp: use qapi_enum_parse() in hmp_migrate_set_parameter, Marc-André Lureau, 2017/08/22
- [Qemu-devel] [PATCH v2 22/54] qapi-introspect: add preprocessor conditions to generated QLit, Marc-André Lureau, 2017/08/22
- [Qemu-devel] [PATCH v2 13/54] qapi: drop the sentinel in enum array, Marc-André Lureau, 2017/08/22
- [Qemu-devel] [PATCH v2 06/54] qapi: introduce qapi_enum_lookup(), Marc-André Lureau, 2017/08/22
- [Qemu-devel] [PATCH v2 12/54] qapi: change enum lookup structure, Marc-André Lureau, 2017/08/22
- [Qemu-devel] [PATCH v2 53/54] qapi: make query-cpu-model-expansion depend on s390 or x86, Marc-André Lureau, 2017/08/22