| Message ID | cover.1552574299.git.crobinso@redhat.com |
|---|---|
| Headers | show |
| Series | tests: qemuxml2argv: support optional arguments | expand |
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@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 3/14/19 11:17 AM, Eric Blake wrote: > 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. > You saw it in the later patches, but just to respond here if anyone else is watching: there is an ARG_END but it's hidden in the macro definitions. - Cole - Cole -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
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); Which may not seem directly compelling, but adding new testInfo fields will be much easier. This will also be a base for a later series to share the VIR_TEST_CAPS infrastructure with qemuxml2xmltest Cole Robinson (21): qemu: add virQEMUCapsSetVList tests: qemuxml2argv: add testInfoSetArgs tests: qemuxml2argv: add va_arg enum handling tests: qemuxml2argv: push ARG_QEMU_CAPS to callers tests: qemuxml2argv: break apart testInitQEMUCaps tests: qemuxml2argv: handle gic with vaargs tests: qemuxml2argv: handle migrate* with varargs tests: qemuxml2argv: handle flags with varargs tests: qemuxml2argv: handle parseFlags with varargs tests: qemuxml2argv: remove DO_TEST_PARSE_FLAGS_ERROR tests: qemuxml2argv: remove unused DO_TEST_CAPS* macros tests: qemuxml2argv: add a comment separating DO_TEST* macros tests: qemuxml2argv: remove unused CAPS migrateFrom tests: qemuxml2argv: use varargs for CAPS flags tests: qemuxml2argv: remove full testInfo initialization tests: qemuxml2argv: centralize CAPS suffix building tests: qemuxml2argv: build capsfile in DO_TEST_CAPS_INTERNAL tests: qemuxml2argv: add testInfoClear tests: qemuxml2argv: move DO_TEST qemuCaps init tests: qemuxml2argv: move DO_CAPS_TEST* qemuCaps init tests: qemuxml2argv: add TEST_INTERNAL src/qemu/qemu_capabilities.c | 14 +- src/qemu/qemu_capabilities.h | 2 + tests/qemuxml2argvtest.c | 273 +++++++++++++++++++++++------------ 3 files changed, 193 insertions(+), 96 deletions(-) -- 2.20.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list