From 087a74e16091a12f09f00466d2d73e3e9ff76a83 Mon Sep 17 00:00:00 2001 From: Michal Privoznik Date: Wed, 27 Mar 2019 17:19:37 +0100 Subject: [PATCH] qemu_capabilities; Drop virQEMUCapsSetVAList MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit There is one specific caller (testInfoSetArgs() in qemuxml2argvtest.c) which expect the va_list argument to change after returning from the virQEMUCapsSetVAList() function. However, since we are passing plain va_list this is not guaranteed. The man page of stdarg(3) says: If ap is passed to a function that uses va_arg(ap,type), then the value of ap is undefined after the return of that function. (ap is a variable of type va_list) I've seen this in action in fact: on i686 the qemuxml2argvtest fails on the second test case because testInfoSetArgs() sees ARG_QEMU_CAPS and calls virQEMUCapsSetVAList to process the capabilities (in this case there's just one QEMU_CAPS_SECCOMP_BLACKLIST). But since the changes are not reflected in the caller, in the next iteration testInfoSetArgs() sees the QEMU capability and not ARG_END. Signed-off-by: Michal Privoznik Reviewed-by: Daniel P. Berrangé --- src/qemu/qemu_capabilities.c | 15 +++------------ src/qemu/qemu_capabilities.h | 2 -- tests/qemuxml2argvtest.c | 6 +++++- 3 files changed, 8 insertions(+), 15 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index c5954edaf0..56228e7a36 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -1663,24 +1663,15 @@ virQEMUCapsSet(virQEMUCapsPtr qemuCaps, } -void -virQEMUCapsSetVAList(virQEMUCapsPtr qemuCaps, - va_list list) -{ - int flag; - - while ((flag = va_arg(list, int)) < QEMU_CAPS_LAST) - ignore_value(virBitmapSetBit(qemuCaps->flags, flag)); -} - - void virQEMUCapsSetList(virQEMUCapsPtr qemuCaps, ...) { va_list list; + int flag; va_start(list, qemuCaps); - virQEMUCapsSetVAList(qemuCaps, list); + while ((flag = va_arg(list, int)) < QEMU_CAPS_LAST) + virQEMUCapsSet(qemuCaps, flag); va_end(list); } diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 7625d754a3..06c7606e2f 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -518,8 +518,6 @@ virQEMUCapsPtr virQEMUCapsNew(void); void virQEMUCapsSet(virQEMUCapsPtr qemuCaps, virQEMUCapsFlags flag) ATTRIBUTE_NONNULL(1); -void virQEMUCapsSetVAList(virQEMUCapsPtr qemuCaps, - va_list list) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); void virQEMUCapsSetList(virQEMUCapsPtr qemuCaps, ...) ATTRIBUTE_NONNULL(1); void virQEMUCapsClear(virQEMUCapsPtr qemuCaps, diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 6e7d1b0b9a..4d6e4b0d39 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -642,6 +642,7 @@ testInfoSetArgs(struct testInfo *info, char *capsarch = NULL; char *capsver = NULL; VIR_AUTOFREE(char *) capsfile = NULL; + int flag; int ret = -1; va_start(argptr, capslatest); @@ -650,7 +651,10 @@ testInfoSetArgs(struct testInfo *info, case ARG_QEMU_CAPS: if (qemuCaps || !(qemuCaps = virQEMUCapsNew())) goto cleanup; - virQEMUCapsSetVAList(qemuCaps, argptr); + + while ((flag = va_arg(argptr, int)) < QEMU_CAPS_LAST) + virQEMUCapsSet(qemuCaps, flag); + break; case ARG_GIC: