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@xxxxxxxxxx>
> ---
> 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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
