qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 2/4] string-input-visitor: Support alternate typ


From: Eric Blake
Subject: Re: [Qemu-devel] [PATCH 2/4] string-input-visitor: Support alternate types
Date: Tue, 2 May 2017 16:37:03 -0500
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.0

On 05/02/2017 03:31 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.
> 
> 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>
> ---

>  
> +/* Support for alternates on string-input-visitor is limited, because
> + * the input string doesn't have any type information.
> + *
> + * Supported alternate member types:
> + * 1) enums
> + * 2) integer types
> + * 3) booleans (but only if the there's no enum variant
> + *    containing "on", "off", "true", or "false" as members)
> + *
> + * UNSUPPORTED alternate member types:
> + * 1) strings
> + * 2) complex types
> + */
> +static void start_alternate(Visitor *v, const char *name,
> +                            GenericAlternate **obj, size_t size,
> +                            unsigned long supported_qtypes, Error **errp)
> +{
> +    StringInputVisitor *siv = to_siv(v);
> +    QType t = QTYPE_QSTRING;

Why do you document string as unsupported, and yet default to
QTYPE_QSTRING?  I don't see how a string is fundamentally different from
an enum (no alternate can have both at the same time, an alternate with
either type will have QTYPE_QSTRING set in supported_qtypes).

> +
> +    if (supported_qtypes & BIT(QTYPE_QBOOL)) {
> +        if (try_parse_bool(siv->string, NULL) == 0) {
> +            t = QTYPE_QBOOL;
> +        }
> +    }
> +
> +    if (supported_qtypes & BIT(QTYPE_QINT)) {
> +        if (parse_str(siv, name, NULL) == 0) {
> +            t = QTYPE_QINT;
> +        }
> +    }
> +
> +    *obj = g_malloc0(size);
> +    (*obj)->type = t;

Should you raise an error if you couldn't match the input with
supported_qtypes, rather than just blindly returning QTYPE_QSTRING?

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]