On 3/14/19 11:23 AM, Eric Blake wrote:
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?),...
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
+ 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.
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
Thanks,
Cole
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
