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

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

>> +    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

> 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