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

[Eric Blake]

On 3/14/19 2:42 PM, Cole Robinson wrote:

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

Do you need some sort of compile-time check that QEMU_CAPS_LAST doesn't
overlap with any other ARG_*?


>>> +    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?),...
>>
> 
> See the while() condition: if we see ARG_END, we exit the loop, so we
> shouldn't ever hit this condition and it's only in the switch to appease
> gcc

Indeed, now that you point it out, it makes sense.

>> 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.
>>
> 
> The double sentinel is actually a requirement of the current code,
> because once we see ARG_QEMU_CAPS, we pass off the va_list to
> virQEMUCapsSetVList which does its own arg processing and uses
> QEMU_CAPS_LAST as a sentinel. We then kick back to this while() loop,
> which sees ARG_END, and completes parsing.
> 
> The ARG_END = QEMU_CAPS_LAST is a bit weird, but it handles the
> DO_TEST(..., NONE) case, which translates to
> 
> testInfoSetArgs(&info, ARG_QEMU_CAPS, NONE, QEMU_CAPS_LAST, ARG_END)
> 
> Sine NONE == QEMU_CAPS_LAST == ARG_END, we finish parsing on
> QEMU_CAPS_LAST. If ARG_END != QEMU_CAPS_LAST, the loop would try to
> interpret QEMU_CAPS_LAST as an ARG_X value, and fail

Clever. May be worth a comment in the code and/or commit message, but
you've convinced me why it works.

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