Re: [Qemu-devel] [PATCH v2 4/4] ARRAY_SIZE: check that argument is an array

[Peter Maydell]

On 19 January 2017 at 08:20, Markus Armbruster <address@hidden> wrote:
> "Michael S. Tsirkin" <address@hidden> writes:
>
>> It's a familiar pattern: some code uses ARRAY_SIZE, then refactoring
>> changes the argument from an array to a pointer to a dynamically
>> allocated buffer.  Code keeps compiling but any ARRAY_SIZE calls now
>> return the size of the pointer divided by element size.
>>
>> Let's add build time checks to ARRAY_SIZE before we allow more
>> of these in the code-base.
>
> Yes, please!
>
>> Signed-off-by: Michael S. Tsirkin <address@hidden>
>> ---
>>  include/qemu/osdep.h | 8 +++++++-
>>  1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
>> index 689f253..24bfda0 100644
>> --- a/include/qemu/osdep.h
>> +++ b/include/qemu/osdep.h
>> @@ -199,7 +199,13 @@ extern int daemon(int, int);
>>  #endif
>>
>>  #ifndef ARRAY_SIZE
>> -#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
>> +/*
>> + * &(x)[0] is always a pointer - if it's same type as x then the argument 
>> is a
>> + * pointer, not an array as expected.
>> + */
>> +#define ARRAY_SIZE(x) ((sizeof(x) / sizeof((x)[0])) + 
>> QEMU_BUILD_BUG_ON_ZERO( \
>> +                        __builtin_types_compatible_p(typeof(x), \
>> +                                                     typeof(&(x)[0]))))
>
> Please break the line near the operator, not within one of its operands:
>
>    #define ARRAY_SIZE(x) ((sizeof(x) / sizeof((x)[0]))                  \
>                           + QEMU_BUILD_BUG_ON_ZERO(                     \
>                                __builtin_types_compatible_p(typeof(x),  \
>                                                             typeof(&(x)[0]))))


The other possible approach to the long-lines issue would be to do what
the Linux kernel does and abstract out a MUST_BE_ARRAY() macro.

thanks
-- PMM