Re: [Qemu-block] [Qemu-devel] [PATCH 2/5] lockable: add QemuLockable

[Paolo Bonzini]

On 03/02/2018 21:44, Richard Henderson wrote:
> On 02/03/2018 07:39 AM, Paolo Bonzini wrote:
>> +/* This function gives an error if an invalid, non-NULL pointer type is 
>> passed
>> + * to QEMU_MAKE_LOCKABLE.  For optimized builds, we can rely on dead-code 
>> elimination
>> + * from the compiler, and give the errors already at link time.
>> + */
>> +#ifdef __OPTIMIZE__
>> +void unknown_lock_type(void *);
>> +#else
>> +static inline void unknown_lock_type(void *unused)
>> +{
>> +    abort();
>> +}
> 
> Since you're using __builtin_choose_expr, I'm surprised you would need to test
> __OPTIMZE__.  The nature of the builtin is such that it *must* eliminate the
> other branch; otherwise the types don't even match up.

__builtin_choose_expr works fine.  However we also have

        return x ? __builtin_choose_expr(..., unknown_lock_type) : NULL;

and if "x" is NULL then its type is a void*, and the LHS will refer to
unknown_lock_type.  I was expecting __always_inline__ to be enough to
avoid this, but apparently this is not the case.

Paolo

> It might be worth using __attribute__((error("message"))) on the function too.
> 
> Otherwise,
> 
> Reviewed-by: Richard Henderson <address@hidden>
> 
> 
> r~
>