On 3/27/19 2:46 PM, Eric Blake wrote:
On 3/27/19 7:02 AM, 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)
however I'd like to hear Eric's opinion before this gets merged.
Passing a va_list *arg as a parameter is doable, but it comes with odd
effects. That is because the C standard permits both of the following
implementation styles:
typedef struct __something *va_list;
typedef struct __something va_list[];
but you get different semantics on what happens if you try to take the
address of a va_list parameter, (C parameter lists undergo pointer
decay, and taking the address of a parameter results in either the
address of a pointer or the address of the first member of an array -
which are different levels of dereferencing and thus different types).
The type 'pointer to va_list' is sane, and the computation of
'&local_va_list' is sane; it is only '¶meter_va_list' that gets you
in trouble. Here's where I demonstrated it further for the qemu list:
https://lists.gnu.org/archive/html/qemu-devel/2017-10/msg00171.html
The only portable way to take va_list from one function to pass to a
va_list * of another function is to va_copy() from the parameter into a
local va_list, then take the address of the local. But if you are the
original owner of a local va_list (because your function took ...
instead of va_list), you don't have to worry about the hoop-jumping
involved to stay portable.
@@ -1680,7 +1680,7 @@ virQEMUCapsSetList(virQEMUCapsPtr qemuCaps, ...)
va_list list;
va_start(list, qemuCaps);
- virQEMUCapsSetVAList(qemuCaps, list);
+ virQEMUCapsSetVAList(qemuCaps, &list);
va_end(list);
This usage is safe, because it is the address of a local variable. I
don't see any instance where you are taking the address of a parameter,
so while the patch is unusual, it doesn't look wrong.
Okay, thanks for detailed explanation. I'll probably go with what Dan
sugested and make virQEMUCapsSetVAList inline then. It looks safer and
more portable.
Michal
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
