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

[Eric Blake]

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.
> 
> 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?

An example program is probably the best to visualize the problem:

$ cat typefun.c
#include <stdio.h>

typedef struct T {
    int i[5];
} T;
typedef struct E {
    int i;
} E;

typedef T list1;
typedef E list2[5];

void bar(const char *prefix, list1 *l1, list2 *l2) {
    printf ("%s: %zu %zu\n", prefix, sizeof(&l1), sizeof(&l2));
}

void foo(list1 l1, list2 l2) {
    printf ("parameter sizes: %zu %zu\n", sizeof(l1), sizeof(l2));
    bar("called with address of parameter", &l1, &l2);
}

int main(void) {
    list1 l1;
    list2 l2;
    printf ("local variable sizes: %zu %zu\n", sizeof(l1), sizeof(l2));
    bar("called with address of local variable", &l1, &l2);
    foo(l1, l2);
    return 0;
}

$ gcc -o typefun -Wall typefun.c
typefun.c: In function ‘foo’:
typefun.c:18:61: warning: ‘sizeof’ on array function parameter ‘l2’ will
return size of ‘E * {aka struct E *}’ [-Wsizeof-array-argument]
     printf ("parameter sizes: %zu %zu\n", sizeof(l1), sizeof(l2));
                                                             ^
typefun.c:17:26: note: declared here
 void foo(list1 l1, list2 l2) {
                          ^~
typefun.c:19:50: warning: passing argument 3 of ‘bar’ from incompatible
pointer type [-Wincompatible-pointer-types]
     bar("called with address of parameter", &l1, &l2);
                                                  ^
typefun.c:13:6: note: expected ‘E (*)[5] {aka struct E (*)[5]}’ but
argument is of type ‘E ** {aka struct E **}’
 void bar(const char *prefix, list1 *l1, list2 *l2) {
      ^~~

$ ./typefun
local variable sizes: 20 20
called with address of local variable: 8 8
parameter sizes: 20 8
called with address of parameter: 8 8

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

signature.asc
Description: OpenPGP digital signature