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

[Eric Blake]

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 '&parameter_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.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org