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

[Eric Blake]

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?

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.

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

signature.asc
Description: OpenPGP digital signature