[libvirt] [PATCH] qemu_capabilities: Pass pointer to va_list in virQEMUCapsSetVAList
Michal Privoznik
mprivozn at redhat.com
Wed Mar 27 15:49:43 UTC 2019
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
More information about the libvir-list
mailing list