Re: [Qemu-devel] [PATCH 2/5] lock-guard: add scoped lock implementation

[Paolo Bonzini <pbonzini@redhat.com>]

On 08/12/2017 16:30, Stefan Hajnoczi wrote:
> On Fri, Dec 08, 2017 at 11:55:50AM +0100, Paolo Bonzini wrote:
> 
> The implementation is somewhat complex.  Please structure the header
> file so the public interfaces are clear and documented.  Move the
> implementation out of the way and mark it private.  That will make it
> easier for someone to figure out "how do I use lock-guard.h".

Right, unfortunately you pretty much have to place it in the header to
guarantee that everything is inlined.

>> diff --git a/include/qemu/lock-guard.h b/include/qemu/lock-guard.h
>> new file mode 100644
>> index 0000000000..e6a83bf9ee
>> --- /dev/null
>> +++ b/include/qemu/lock-guard.h
>> @@ -0,0 +1,103 @@
>> +#ifndef QEMU_LOCK_GUARD_H
>> +#define QEMU_LOCK_GUARD_H 1
>> +
>> +typedef void QemuLockGuardFunc(void *);
>> +typedef struct QemuLockGuard {
>> +    QemuLockGuardFunc *p_lock_fn, *p_unlock_fn;
>> +    void *lock;
>> +    int locked;
> 
> bool?

Yes.

>> +#define QEMU_WITH_LOCK(type, name, lock)                                   \
>> +    for (QEMU_LOCK_GUARD(type, name, lock);                                \
>> +         qemu_lock_guard_is_taken(&name);                                  \
>> +         qemu_lock_guard_unlock(&name))
> 
> I don't understand the need for the qemu_lock_guard_is_taken(&name)
> condition, why not do the following?
> 
>   for (QEMU_LOCK_GUARD(type, name, lock);
>        ;
>        qemu_lock_guard_unlock(&name))

Because that would be an infinite loop. :)

Paolo

> Also, the for loop means that break statements do not work inside
> QEMU_WITH_LOCK() { ... }.  This needs to be documented.