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.
