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

[Stefan Hajnoczi <stefanha@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1449 bytes --]

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.

Maybe I've missed something?

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]