On Thu, Sep 30, 2021 at 6:29 AM Tim Wiederhake <twiederh@redhat.com> wrote:
Locks a virMutex on creation and unlocks it in its destructor.
The VIR_LOCK_GUARD macro is used instead of "g_autoptr(virLockGuard)" to work around a clang issue (see https://bugs.llvm.org/show_bug.cgi?id=3888 and https://bugs.llvm.org/show_bug.cgi?id=43482).
Typical usage:
void function(virMutex *m) { VIR_LOCK_GUARD lock = virLockGuardNew(m); /* `m` is locked, and released automatically on scope exit */
... while (expression) { VIR_LOCK_GUARD lock2 = virLockGuardNew(...); /* similar */ } }
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/libvirt_private.syms | 3 +++ src/util/virthread.c | 26 ++++++++++++++++++++++++++ src/util/virthread.h | 11 +++++++++++ 3 files changed, 40 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 6de9d9aef1..f41674781d 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -3373,6 +3373,9 @@ virCondInit; virCondSignal; virCondWait; virCondWaitUntil; +virLockGuardFree; +virLockGuardNew; +virLockGuardUnlock; virMutexDestroy; virMutexInit; virMutexInitRecursive; diff --git a/src/util/virthread.c b/src/util/virthread.c index e89c1a09fb..a5a948985f 100644 --- a/src/util/virthread.c +++ b/src/util/virthread.c @@ -96,6 +96,32 @@ void virMutexUnlock(virMutex *m) pthread_mutex_unlock(&m->lock); }
+virLockGuard *virLockGuardNew(virMutex *m) +{ + virLockGuard *l = g_new0(virLockGuard, 1); + l->mutex = m; + + virMutexLock(l->mutex); + return l; +}
I realize that I'm jumping into the discussion in the third iteration of these patches, so I apologize if this has already been discussed (though I did look back at the earlier versions but didn't see anything). Is there a reason that we're dynamically allocating the lock guard rather than just allocating it as a local variable on the stack? (i.e. using g_autoptr() vs. g_auto()).
+ +void virLockGuardFree(virLockGuard *l) +{ + if (!l) + return; + + virLockGuardUnlock(l); + g_free(l); +} + +void virLockGuardUnlock(virLockGuard *l) +{ + if (!l) + return; + + virMutexUnlock(g_steal_pointer(&l->mutex)); +} +
int virRWLockInit(virRWLock *m) { diff --git a/src/util/virthread.h b/src/util/virthread.h index 55c8263ae6..d896a6ad3a 100644 --- a/src/util/virthread.h +++ b/src/util/virthread.h @@ -31,6 +31,11 @@ struct virMutex { pthread_mutex_t lock; };
+typedef struct virLockGuard virLockGuard; +struct virLockGuard { + virMutex *mutex; +}; + typedef struct virRWLock virRWLock; struct virRWLock { pthread_rwlock_t lock; @@ -121,6 +126,12 @@ void virMutexLock(virMutex *m); void virMutexUnlock(virMutex *m);
+virLockGuard *virLockGuardNew(virMutex *m); +void virLockGuardFree(virLockGuard *l); +void virLockGuardUnlock(virLockGuard *l); +G_DEFINE_AUTOPTR_CLEANUP_FUNC(virLockGuard, virLockGuardFree); +#define VIR_LOCK_GUARD g_autoptr(virLockGuard) G_GNUC_UNUSED
As long as we're essentially required to use a custom macro for the type declaration, I almost feel like we should just go all the way and put the variable declaration inside the macro so that the usage looks more like: void function(virMutex *m) { VIR_LOCK_GUARD(m); /* variable gets declared and assigned here */ /* m is locked within function and released on exit ... } Maybe even use a name like VIR_SCOPED_LOCK?
+ int virRWLockInit(virRWLock *m) G_GNUC_WARN_UNUSED_RESULT; void virRWLockDestroy(virRWLock *m);
-- 2.31.1