On 10/02/2017 09:30 AM, Eric Blake wrote: > On 10/02/2017 12:46 AM, Markus Armbruster wrote: > >>>>> + /* >>>>> + * 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.
When I first wrote that, I didn't know where it mattered in practice; now I do. Here's a gcc non-bug: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=14557 that documents a workaround of using a macro va_ptr() because there ARE existing modern platforms that use an array type for va_list: https://sourceware.org/ml/newlib/2017/msg01302.html I don't know if va_ptr() is any more elegant than the va_copy() used in my code motion. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature