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 <ehabk...@redhat.com> > ---
>
> +/* 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
signature.asc
Description: OpenPGP digital signature
