Re: [Qemu-devel] [PATCH 2/8] qapi script: report error for default case in union visit

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1320 bytes --]

On 11/06/2013 12:33 PM, Wenchao Xia wrote:
> It is possible to reach default case, when an union have a enum
> discriminator, so don't abort() but report the error message.
> 
> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
> ---
>  scripts/qapi-visit.py |   14 ++++++++++++--
>  1 files changed, 12 insertions(+), 2 deletions(-)

I still think this is not ideal.  You are proposing a runtime error:

> +    # Tell caller the value is invalid, since the discriminator value maybe an
> +    # unmapped enum value.
>      ret += mcgen('''
>              default:
> -                abort();
> +                error_setg(&err,
> +                           "Invalid discriminator value %(pi)s for %(name)s",
> +                           (*obj)->kind);
> +                break;
>              }

whereas I'm requesting a compile-time error - it is much easier for
maintenance reasons to have the generator require up-front that all enum
values are covered, and loudly complain if an enum type is extended
without also extending the use of that enum type in a union, than it is
to silently generate a runtime error and wait for the bug reports
several weeks down the road.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]