Re: [Qemu-devel] [PATCH v4 05/22] qobject: Simplify qobject_from_jsonv()

[Markus Armbruster]

Eric Blake <address@hidden> writes:

> On 08/09/2017 02:59 AM, Markus Armbruster wrote:
>> Eric Blake <address@hidden> writes:
>> 
>>> qobject_from_jsonv() was unusual in that it took a va_list*, instead
>>> of the more typical va_list; this was so that callers could pass
>>> NULL to avoid % interpolation.  While this works under the hood, it
>>> is awkward for callers, so move the magic into qjson.c rather than
>>> in the public interface, and finally improve the documentation of
>>> qobject_from_jsonf().
>>>
>
>>> -    /* Going through qobject ensures we escape strings properly.
>>> -     * This seemingly unnecessary copy is required in case va_list
>>> -     * is an array type.
>>> -     */
>>> -    va_copy(ap_copy, ap);
>>> -    qobj = qobject_from_jsonv(fmt, &ap_copy, &error_abort);
>>> -    va_end(ap_copy);
>>> +    /* Going through qobject ensures we escape strings properly. */
>>> +    qobj = qobject_from_jsonv(fmt, ap);
>>>      qstr = qobject_to_json(qobj);
>>>
>>>      /*
>> 
>> Wait!  Oh, the va_copy() moves iinto qobject_from_jsonv().  Okay, I
>> guess.
[...]
>>> +
>>> +    /*
>>> +     * This seemingly unnecessary copy is required in case va_list
>>> +     * is an array type.
>>> +     */
>> 
>> --verbose?
>>
>>> +    va_copy(ap_copy, ap);
>>> +    obj = qobject_from_json_internal(string, &ap_copy, &error_abort);
>>> +    va_end(ap_copy);
>
> Code motion. But if the comment needs to be more verbose in the
> destination than it was on the source, the rationale is that C99/POSIX
> allows 'typedef something va_list[]' (that is, where va_list is an array
> of some other type), although I don't know of any modern OS that
> actually defines it like that.  Based on C pointer-decay rules, '&ap'
> has a different type based on whether va_list was a struct/pointer or an
> array type, when 'va_list ap' was passed as a parameter; so we can't
> portably use qobject_from_json_internal(string, &ap, &error_abort).  The
> va_copy() is what lets us guarantee that &ap_list is a pointer to a
> va_list regardless of the type of va_list (because va_copy was declared
> locally, rather than in a parameter list, and is therefore not subject
> to pointer decay), and NOT an accidental pointer to first element of the
> va_list array on platforms where va_list is an array.

I'm dense this Monday morning --- I still can't see where exactly
passing &ap directly goes wrong.

Two cases:

1. va_list is a typedef name for a non-array type T.

2. va_list is a typedef name for an array type E[].

What are the types of actual argument &ap and formal parameter va_list
*ap in either case?

How exactly does case 2 break?