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

[Paolo Bonzini]

On 29/01/2018 12:42, Stefan Hajnoczi wrote:
> On Thu, Jan 25, 2018 at 06:59:46PM +0100, Paolo Bonzini wrote:
>> +struct QemuLockable {
>> +    void *object;
>> +    QemuLockUnlockFunc *lock;
>> +    QemuLockUnlockFunc *unlock;
>> +};
> ...
>> +/* In C, compound literals have the lifetime of an automatic variable.
>> + * In C++ it would be different, but then C++ wouldn't need QemuLockable
>> + * either...
>> + */
>> +#define QEMU_MAKE_LOCKABLE_(x) qemu_make_lockable((x), &(QemuLockable) {    
>> \
>> +        .object = (x),                               \
>> +        .lock = QEMU_LOCK_FUNC(x),                   \
>> +        .unlock = QEMU_UNLOCK_FUNC(x),               \
>> +    })
> 
> After all these tricks we still end up with a struct containing function
> pointers.  That's a little sad because the power of generics is
> specializing code at compile time.
> 
> IMO the generics usage here doesn't have a big pay-off.  The generated
> code is more or less the same as without generics!
> 
> It makes me wonder if the API would be more maintainable with:
> 
>   typedef struct {
>       QemuLockUnlockFunc *lock;
>       QemuLockUnlockFunc *unlock;
>   } LockableOps;
> 
>   extern const LockableOps co_mutex_lockable_ops;
>   extern const LockableOps qemu_spin_lockable_ops;
>   ...
> 
> The user passes in the appropriate LockableOps instance for their type
> and generics are not needed.
> 
> This approach means future changes do not require digging through the
> macros to understand how this stuff works.

The difference between the two is that, once we use it for lock guards,
the compiler is able to see constant function pointers and inline
everything.  If you use LockableOps, it doesn't.

Paolo

> Maybe I've missed something?
> 
> Stefan
>