From 9e8f07b1563ae215be11e8331cf11e4a93703ee8 Mon Sep 17 00:00:00 2001 From: Xavier Claessens Date: Mon, 15 Jan 2024 12:44:13 -0500 Subject: [PATCH] Add G_*_AUTO_LOCK macros It is common to declare a mutex locker variable and to not use it in the scope. That causes clang to warn about unused variable which is not entirely true since the cleanup function IS the intended usage. Work around that issue with a new macro that uses G_GNUC_UNUSED and has the extra advantage of being less verbose. Fixes: #3223. --- glib/gthread.c | 14 ++++ glib/gthread.h | 194 ++++++++++++++++++++++++++++++++++++++++++- glib/tests/autoptr.c | 46 ++++++++++ 3 files changed, 251 insertions(+), 3 deletions(-) diff --git a/glib/gthread.c b/glib/gthread.c index e7012fd11..b39acc475 100644 --- a/glib/gthread.c +++ b/glib/gthread.c @@ -142,6 +142,20 @@ * %G_LOCK_DEFINE. */ +/** + * G_AUTO_LOCK: + * @name: the name of the lock + * + * Works like [func@GLib.MUTEX_AUTO_LOCK], but for a lock defined with + * [func@GLib.LOCK_DEFINE]. + * + * This feature is only supported on GCC and clang. This macro is not defined on + * other compilers and should not be used in programs that are intended to be + * portable to those compilers. + * + * Since: 2.80 + */ + /* GMutex Documentation {{{1 ------------------------------------------ */ /** diff --git a/glib/gthread.h b/glib/gthread.h index 1b5a89a68..3cfa28f84 100644 --- a/glib/gthread.h +++ b/glib/gthread.h @@ -139,6 +139,10 @@ struct _GOnce # define G_TRYLOCK(name) g_mutex_trylock (&G_LOCK_NAME (name)) #endif /* !G_DEBUG_LOCKS */ +#ifdef g_autoptr +#define G_AUTO_LOCK(name) G_MUTEX_AUTO_LOCK (&G_LOCK_NAME (name), g__##name##_locker) +#endif /* g_autoptr */ + GLIB_AVAILABLE_IN_2_32 GThread * g_thread_ref (GThread *thread); GLIB_AVAILABLE_IN_2_32 @@ -339,7 +343,7 @@ typedef void GMutexLocker; * * // Code with mutex locked here * - * if (cond) + * if (condition) * // No need to unlock * return; * @@ -350,6 +354,10 @@ typedef void GMutexLocker; * } * ]| * + * Note that it is common for the declared variable to not be used in the scope, + * which causes some compilers to warn. That can be avoided by using + * `G_GNUC_UNUSED` or, since 2.80, [func@GLib.MUTEX_AUTO_LOCK]. + * * Returns: a #GMutexLocker * Since: 2.44 */ @@ -378,6 +386,49 @@ g_mutex_locker_free (GMutexLocker *locker) g_mutex_unlock ((GMutex *) locker); } +/** + * G_MUTEX_AUTO_LOCK: + * @mutex: a [type@GLib.Mutex] + * @var: a variable name to be declared + * + * Declare a [type@GLib.MutexLocker] variable with `g_autoptr()` and lock the + * mutex. The mutex will be unlocked automatically when leaving the scope. The + * variable is declared with `G_GNUC_UNUSED` to avoid compiler warning if it is + * not used in the scope. + * + * This feature is only supported on GCC and clang. This macro is not defined on + * other compilers and should not be used in programs that are intended to be + * portable to those compilers. + * + * Note that this should be used in a place where it is allowed to declare a + * variable, which could be before any statement in the case + * `-Wdeclaration-after-statement` is used, or C standard prior to C99. + * + * ```c + * { + * G_MUTEX_AUTO_LOCK (&obj->mutex, locker); + * + * obj->stuff_with_lock (); + * if (condition) + * { + * // No need to unlock + * return; + * } + * + * // Unlock before end of scope + * g_clear_pointer (&locker, g_mutex_locker_free); + * obj->stuff_without_lock (); + * } + * ``` + * + * Since: 2.80.0 + */ +#ifdef g_autoptr +#define G_MUTEX_AUTO_LOCK(mutex, var) \ + GLIB_AVAILABLE_MACRO_IN_2_80 g_autoptr (GMutexLocker) \ + G_GNUC_UNUSED var = g_mutex_locker_new (mutex) +#endif /* g_autoptr */ + /** * GRecMutexLocker: * @@ -414,7 +465,7 @@ typedef void GRecMutexLocker; * * // Code with rec_mutex locked here * - * if (cond) + * if (condition) * // No need to unlock * return; * @@ -425,6 +476,10 @@ typedef void GRecMutexLocker; * } * ]| * + * Note that it is common for the declared variable to not be used in the scope, + * which causes some compilers to warn. That can be avoided by using + * `G_GNUC_UNUSED` or, since 2.80, [func@GLib.REC_MUTEX_AUTO_LOCK]. + * * Returns: a #GRecMutexLocker * Since: 2.60 */ @@ -457,6 +512,49 @@ g_rec_mutex_locker_free (GRecMutexLocker *locker) } G_GNUC_END_IGNORE_DEPRECATIONS +/** + * G_REC_MUTEX_AUTO_LOCK: + * @mutex: a [type@GLib.RecMutex] + * @var: a variable name to be declared + * + * Declare a [type@GLib.RecMutexLocker] variable with `g_autoptr()` and lock the + * mutex. The mutex will be unlocked automatically when leaving the scope. The + * variable is declared with `G_GNUC_UNUSED` to avoid compiler warning if it is + * not used in the scope. + * + * This feature is only supported on GCC and clang. This macro is not defined on + * other compilers and should not be used in programs that are intended to be + * portable to those compilers. + * + * Note that this should be used in a place where it is allowed to declare a + * variable, which could be before any statement in the case + * `-Wdeclaration-after-statement` is used, or C standard prior to C99. + * + * ```c + * { + * G_REC_MUTEX_AUTO_LOCK (&obj->rec_mutex, locker); + * + * obj->stuff_with_lock (); + * if (condition) + * { + * // No need to unlock + * return; + * } + * + * // Unlock before end of scope + * g_clear_pointer (&locker, g_rec_mutex_locker_free); + * obj->stuff_without_lock (); + * } + * ``` + * + * Since: 2.80.0 + */ +#ifdef g_autoptr +#define G_REC_MUTEX_AUTO_LOCK(mutex, var) \ + GLIB_AVAILABLE_MACRO_IN_2_80 g_autoptr (GRecMutexLocker) \ + G_GNUC_UNUSED var = g_rec_mutex_locker_new (mutex) +#endif /* g_autoptr */ + /** * GRWLockWriterLocker: * @@ -520,7 +618,7 @@ typedef void GRWLockWriterLocker; * if (self->array == NULL) * self->array = g_ptr_array_new (); * - * if (cond) + * if (condition) * // No need to unlock * return; * @@ -535,6 +633,10 @@ typedef void GRWLockWriterLocker; * } * ]| * + * Note that it is common for the declared variable to not be used in the scope, + * which causes some compilers to warn. That can be avoided by using + * `G_GNUC_UNUSED` or, since 2.80, [func@GLib.RW_LOCK_WRITER_AUTO_LOCK]. + * * Returns: a #GRWLockWriterLocker * Since: 2.62 */ @@ -568,6 +670,49 @@ g_rw_lock_writer_locker_free (GRWLockWriterLocker *locker) } G_GNUC_END_IGNORE_DEPRECATIONS +/** + * G_RW_LOCK_WRITER_AUTO_LOCK: + * @mutex: a [type@GLib.RWLock] + * @var: a variable name to be declared + * + * Declare a [type@GLib.RWLockWriterLocker] variable with `g_autoptr()` and lock + * for writing. The mutex will be unlocked automatically when leaving the scope. + * The variable is declared with `G_GNUC_UNUSED` to avoid compiler warning if it + * is not used in the scope. + * + * This feature is only supported on GCC and clang. This macro is not defined on + * other compilers and should not be used in programs that are intended to be + * portable to those compilers. + * + * Note that this should be used in a place where it is allowed to declare a + * variable, which could be before any statement in the case + * `-Wdeclaration-after-statement` is used, or C standard prior to C99. + * + * ```c + * { + * G_RW_LOCK_WRITER_AUTO_LOCK (&obj->rw_lock, locker); + * + * obj->stuff_with_lock (); + * if (condition) + * { + * // No need to unlock + * return; + * } + * + * // Unlock before end of scope + * g_clear_pointer (&locker, g_rw_lock_writer_locker_free); + * obj->stuff_without_lock (); + * } + * ``` + * + * Since: 2.80.0 + */ +#ifdef g_autoptr +#define G_RW_LOCK_WRITER_AUTO_LOCK(mutex, var) \ + GLIB_AVAILABLE_MACRO_IN_2_80 g_autoptr (GRWLockWriterLocker) \ + G_GNUC_UNUSED var = g_rw_lock_writer_locker_new (mutex) +#endif /* g_autoptr */ + /** * GRWLockReaderLocker: * @@ -623,6 +768,49 @@ g_rw_lock_reader_locker_free (GRWLockReaderLocker *locker) } G_GNUC_END_IGNORE_DEPRECATIONS +/** + * G_RW_LOCK_READER_AUTO_LOCK: + * @mutex: a [type@GLib.RWLock] + * @var: a variable name to be declared + * + * Declare a [type@GLib.RWLockReaderLocker] variable with `g_autoptr()` and lock + * for reading. The mutex will be unlocked automatically when leaving the scope. + * The variable is declared with `G_GNUC_UNUSED` to avoid compiler warning if it + * is not used in the scope. + * + * This feature is only supported on GCC and clang. This macro is not defined on + * other compilers and should not be used in programs that are intended to be + * portable to those compilers. + * + * Note that this should be used in a place where it is allowed to declare a + * variable, which could be before any statement in the case + * `-Wdeclaration-after-statement` is used, or C standard prior to C99. + * + * ```c + * { + * G_RW_LOCK_READER_AUTO_LOCK (&obj->rw_lock, locker); + * + * obj->stuff_with_lock (); + * if (condition) + * { + * // No need to unlock + * return; + * } + * + * // Unlock before end of scope + * g_clear_pointer (&locker, g_rw_lock_reader_locker_free); + * obj->stuff_without_lock (); + * } + * ``` + * + * Since: 2.80.0 + */ +#ifdef g_autoptr +#define G_RW_LOCK_READER_AUTO_LOCK(mutex, var) \ + GLIB_AVAILABLE_MACRO_IN_2_80 g_autoptr (GRWLockReaderLocker) \ + G_GNUC_UNUSED var = g_rw_lock_reader_locker_new (mutex) +#endif /* g_autoptr */ + G_END_DECLS #endif /* __G_THREAD_H__ */ diff --git a/glib/tests/autoptr.c b/glib/tests/autoptr.c index e10c95c9d..4ed27f52b 100644 --- a/glib/tests/autoptr.c +++ b/glib/tests/autoptr.c @@ -399,6 +399,12 @@ test_g_mutex_locker (void) g_mutex_init (&mutex); + if (TRUE) + { + /* val is unused in this scope but compiler should not warn. */ + G_MUTEX_AUTO_LOCK (&mutex, val); + } + if (TRUE) { g_autoptr(GMutexLocker) val = g_mutex_locker_new (&mutex); @@ -442,6 +448,12 @@ test_g_rec_mutex_locker (void) g_rec_mutex_init (&rec_mutex); + if (TRUE) + { + /* val is unused in this scope but compiler should not warn. */ + G_REC_MUTEX_AUTO_LOCK (&rec_mutex, val); + } + if (TRUE) { g_autoptr(GRecMutexLocker) val = g_rec_mutex_locker_new (&rec_mutex); @@ -492,6 +504,18 @@ test_g_rw_lock_lockers (void) g_rw_lock_init (&lock); + if (TRUE) + { + /* val is unused in this scope but compiler should not warn. */ + G_RW_LOCK_WRITER_AUTO_LOCK (&lock, val); + } + + if (TRUE) + { + /* val is unused in this scope but compiler should not warn. */ + G_RW_LOCK_READER_AUTO_LOCK (&lock, val); + } + if (TRUE) { g_autoptr(GRWLockWriterLocker) val = g_rw_lock_writer_locker_new (&lock); @@ -533,6 +557,27 @@ test_g_rw_lock_lockers (void) g_rw_lock_clear (&lock); } +G_LOCK_DEFINE (test_g_auto_lock); + +static void +test_g_auto_lock (void) +{ + GThread *thread; + + if (TRUE) + { + G_AUTO_LOCK (test_g_auto_lock); + + /* Verify that the mutex is actually locked */ + thread = g_thread_new ("mutex locked", mutex_locked_thread, &G_LOCK_NAME (test_g_auto_lock)); + g_thread_join (thread); + } + + /* Verify that the mutex is unlocked again */ + thread = g_thread_new ("mutex unlocked", mutex_unlocked_thread, &G_LOCK_NAME (test_g_auto_lock)); + g_thread_join (thread); +} + static void test_g_cond (void) { @@ -785,6 +830,7 @@ main (int argc, gchar *argv[]) g_test_add_func ("/autoptr/g_mutex_locker", test_g_mutex_locker); g_test_add_func ("/autoptr/g_rec_mutex_locker", test_g_rec_mutex_locker); g_test_add_func ("/autoptr/g_rw_lock_lockers", test_g_rw_lock_lockers); + g_test_add_func ("/autoptr/g_auto_lock", test_g_auto_lock); g_test_add_func ("/autoptr/g_cond", test_g_cond); g_test_add_func ("/autoptr/g_timer", test_g_timer); g_test_add_func ("/autoptr/g_time_zone", test_g_time_zone);