[libvirt] [PATCH 03/21] tests: qemuxml2argv: add va_arg enum handling

[Eric Blake]

On 3/14/19 9:43 AM, Cole Robinson wrote:
> This establishes a pattern that will allow us to make test macros
> more general purpose, by taking optional arguments. The general
> format will be:
> 
> DO_TEST_FULL(...
>              ARG_FOO, <value1>,
>              ARG_BAR, <value2>)
> 
> ARG_X are just enum values that we look for in the va_args and know
> how to interpret.
> 
> Implement this for the existing implicit qemuCaps va_args
> 
> Signed-off-by: Cole Robinson <crobinso at redhat.com>
> ---
>  tests/qemuxml2argvtest.c | 28 +++++++++++++++++++++++++---
>  1 file changed, 25 insertions(+), 3 deletions(-)

> +typedef enum {
> +    ARG_QEMU_CAPS = 1,
> +
> +    ARG_END = QEMU_CAPS_LAST,
> +} testInfoArgNames;
> +

Reading this after my cover letter reply:
Oh, so you _do_ have a sentinel...

>  static int
>  testInfoSetArgs(struct testInfo *info, ...)
>  {
>      va_list argptr;
> -    int ret = 0;
> +    testInfoArgNames argname;
> +    int ret = -1;
>  
>      va_start(argptr, info);
> -    virQEMUCapsSetVList(info->qemuCaps, argptr);
> +    while ((argname = va_arg(argptr, int)) < ARG_END) {
> +        switch (argname) {
> +        case ARG_QEMU_CAPS:
> +            virQEMUCapsSetVList(info->qemuCaps, argptr);
> +            break;
> +
> +        case ARG_END:
> +        default:
> +            fprintf(stderr, "Unexpected test info argument");

...and you are handling it (except that you ALWAYS handle it by printing
an error, is that intentional?),...

> +            goto cleanup;
> +        }
> +    }
> +
> +    ret = 0;
> + cleanup:
>      va_end(argptr);
>      return ret;
>  }
> @@ -821,7 +842,8 @@ mymain(void)
>          }; \
>          if (testInitQEMUCaps(&info, gic) < 0) \
>              return EXIT_FAILURE; \
> -        if (testInfoSetArgs(&info, __VA_ARGS__, QEMU_CAPS_LAST) < 0) \
> +        if (testInfoSetArgs(&info, ARG_QEMU_CAPS, \
> +                            __VA_ARGS__, QEMU_CAPS_LAST, ARG_END) < 0) \

...and you are merely ensuring that the macro supplies the sentinel
automatically, instead of the users having to be aware of it. Works
because all users are calling a macro rather than the direct function.

In fact, for this patch, you are supplying a double-sentinel, and I'm
suspecting (without reading ahead) that later patches improve things as
you add more ARG_ markers, and replacing QEMU_CAPS_LAST (which is now
identical to ARG_END, and confusingly given twice) with something more
obvious.

Okay, you can ignore my noise on the cover letter.

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