Re: [PATCH 00/21] tests: qemuxml2argv: support optional arguments

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 3/14/19 9:43 AM, Cole Robinson wrote:
> Right now in qemuxml2argv we have a proliferation of DO_*TEST* macros.
> They essentially fill in data in the testInfo struct and invoke the
> test function.
> 
> There's several bits of data that we want to specify for a small
> subset of tests: flags, parseFlags, migrateFrom/migrateFd, gic. The
> base macros need to handle these in their argument lists, and we
> provide many convenience macros that fill in default values for the
> less common parameters. Any time we want to add a new bit of data
> though, the base macros are extended, and every caller of those
> needs to be adjusted, and we are faced with the question of whether
> to extend the combinatorial explosion of convenience macros which
> have only a subset of options exposed.
> 
> This series adds a testInfoSetArgs which uses va_args to make these
> mandatory parameters optional. The general format is:
> 
>     DO_TEST_FULL(...
>                  ARG_FOO, foovalue,
>                  ARG_BAR, barvalue, ...)
> 
> Specifically, one of the migrate tests went from:
> 
>     DO_TEST_FULL("restore-v2", "exec:cat", 7, 0, 0, GIC_NONE, NONE);
> 
> to:
> 
>     DO_TEST_FULL("restore-v2",
>                  ARG_MIGRATE_FROM, "exec:cat",
>                  ARG_MIGRATE_FD, 7,
>                  ARG_QEMU_CAPS, NONE);

Umm, what's your sentinel value here? It is undefined behavior to call
va_arg more times than there are arguments present, but without some
sort of sentinel, you don't know when to stop calling va_arg.

The overall idea is nice, but I think you need an ARG_END sentinel.

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



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux