[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 3/3] string-input-visitor: Support alternate
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH v2 3/3] string-input-visitor: Support alternate types |
Date: |
Fri, 5 May 2017 15:53:40 -0500 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.0 |
On 05/05/2017 03:11 PM, Eduardo Habkost wrote:
> When parsing alternates from a string, there are some limitations in
> what we can do, but it is a valid use case in some situations. We can
> support booleans, integer types, and enums.
Stale comment, given...
>
> This will be used to support 'feature=force' in -cpu options, while
> keeping 'feature=on|off|true|false' represented as boolean values.
>
> Signed-off-by: Eduardo Habkost <address@hidden>
> ---
> Changes v1 -> v2:
> * Updated string_input_visitor_new() documentation
> to mention alternate support (Markus Armbruster)
> * Detect ambiguous alternates at runtime. Test case included.
> * Removed support for integers. We don't need it yet, and
...this.
> it would require sorting out the parse_str() mess.
> * Change supported_qtypes to uint32_t (Eric Blake)
> * Update tests/qapi-schema/qapi-schema-test.out to match
> qapi-schema-test.json updates
> (Eric Blake)
> * Code indentation fix (Markus Armbruster)
> * Use &error_abort on test cases instead of g_assert(!err)
> (Markus Armbruster)
> ---
>
> +/*
> + * Check if alternate type string representation is ambiguous and
> + * can't be parsed by StringInputVisitor
> + */
> +static bool ambiguous_alternate(uint32_t qtypes, const char *const
> enum_table[])
> +{
> + uint32_t non_str_qtypes = qtypes & ~(1U << QTYPE_QSTRING);
> +
> + if ((qtypes & (1U << QTYPE_QSTRING)) && !enum_table && non_str_qtypes) {
> + return true;
> + }
> +
> + if (qtypes & (1U << QTYPE_QBOOL)) {
> + const char *const *e;
> + /*
> + * If the string representation of enum members can be parsed as
> + * booleans, the alternate string representation is ambiguous.
> + */
> + for (e = enum_table; e && *e; e++) {
> + if (try_parse_bool(*e, NULL) == 0) {
> + return true;
> + }
> + }
> + }
> +
> + return false;
> +}
Seems okay for detecting ambiguity, but it is a runtime cost (one that
you will run every single time you parse, even though the answer will be
the same every single time you run it); I still think doing it at QAPI
compile time will be more efficient in the long run. And as this is the
only use of enum_table added in 2/3, I'm still not sold on needing that
patch.
> +
> +static void start_alternate(Visitor *v, const char *name,
> + GenericAlternate **obj, size_t size,
> + uint32_t qtypes, const char *const enum_table[],
> + Error **errp)
> +{
> + /*
> + * Enum types are represented as QTYPE_QSTRING, so this is
> + * the default. Actual parsing of the string as an enum is
> + * done by visit_type_<EnumType>(), which is called just
> + * after visit_start_alternate().
> + */
> + QType qtype = QTYPE_QSTRING;
> + uint32_t unsupported_qtypes = qtypes & ~((1U << QTYPE_QSTRING) |
> + (1U << QTYPE_QBOOL));
> + StringInputVisitor *siv = to_siv(v);
> +
> + if (ambiguous_alternate(qtypes, enum_table)) {
> + error_setg(errp, "Can't parse ambiguous alternate type");
> + return;
> + }
> +
> + if (unsupported_qtypes) {
> + error_setg(errp, "Can't parse %s' alternate member",
> + QType_lookup[ctz32(unsupported_qtypes)]);
> + return;
> + }
> +
> + if ((qtypes & (1U << QTYPE_QBOOL)) &&
> + try_parse_bool(siv->string, NULL) == 0) {
> + qtype = QTYPE_QBOOL;
> + }
> +
> + *obj = g_malloc0(size);
> + (*obj)->type = qtype;
Slightly simpler for ignoring int for now, while still something we
could add in later. I've been wanting to have an alternate for
'int'/'str' for InetAddress port, since we want to support named ports
but most often use just integers. On the command line, port=1 is fine,
but in QMP, we currently have to spell it port="1". That's a case where
we'd allow a pairing of any string with an integer, rather than just an
enum.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
- Re: [Qemu-devel] [PATCH v2 1/3] visitor: Add 'supported_qtypes' parameter to visit_start_alternate(), (continued)
[Qemu-devel] [PATCH v2 2/3] qapi: Add enum_table[] parameter to start_alternate, Eduardo Habkost, 2017/05/05
[Qemu-devel] [PATCH v2 3/3] string-input-visitor: Support alternate types, Eduardo Habkost, 2017/05/05
- Re: [Qemu-devel] [PATCH v2 3/3] string-input-visitor: Support alternate types,
Eric Blake <=
Re: [Qemu-devel] [PATCH v2 3/3] string-input-visitor: Support alternate types, Markus Armbruster, 2017/05/10
Re: [Qemu-devel] [PATCH v2 0/3] string-input-visitor: Support enum/bool alternate types, no-reply, 2017/05/05