Re: [PATCH] qemu_capabilities: Pass pointer to va_list in virQEMUCapsSetVAList

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

 



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



[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