On Wed, Mar 27, 2019 at 01:02:07PM +0100, Andrea Bolognani wrote: > On Wed, 2019-03-27 at 10:50 +0100, Michal Privoznik wrote: > > There is one specific caller (testInfoSetArgs() in > > qemuxml2argvtest.c) which expect the va_list argument to change > > s/ / / > > > 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 callse virQEMUCapsSetVAList to process the > > s/callse/calls/ > > > 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. > > s/qemu/QEMU/ > > > > > Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx> > > --- > > > > This passed successfully on x86_64 and i686 in my testing. > > > > src/qemu/qemu_capabilities.c | 6 +++--- > > src/qemu/qemu_capabilities.h | 2 +- > > tests/qemuxml2argvtest.c | 2 +- > > 3 files changed, 5 insertions(+), 5 deletions(-) > > The changes look okay to me, so > > Reviewed-by: Andrea Bolognani <abologna@xxxxxxxxxx> > > however I'd like to hear Eric's opinion before this gets merged. I'll trust Michal when he says it works, but it feels a bit odd to be passing a pointer to a va_list around. Personally I would have just inlined virQEMUCapsSetVAList() as it is only 2 lines of code. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
