12.12.2013 18:52, Stefan Hajnoczi wrote:
> The GStaticMutex API was deprecated in glib 2.32. We cannot switch over
> to GMutex unconditionally since we would drop support for older glib
> versions. But the deprecated API warnings during build are annoying so
> use static GMutex when possible.
>
> Signed-off-by: Stefan Hajnoczi <stefa...@redhat.com>
> ---
> trace/simple.c | 45 ++++++++++++++++++++++++++++++++++++++-------
> 1 file changed, 38 insertions(+), 7 deletions(-)
>
> diff --git a/trace/simple.c b/trace/simple.c
> index 1e3f691..941f7ea 100644
> --- a/trace/simple.c
> +++ b/trace/simple.c
> @@ -39,7 +39,11 @@
> * Trace records are written out by a dedicated thread. The thread waits for
> * records to become available, writes them out, and then waits again.
> */
> +#if GLIB_CHECK_VERSION(2, 32, 0)
> +static GMutex trace_lock;
> +#else
> static GStaticMutex trace_lock = G_STATIC_MUTEX_INIT;
> +#endif
>
> /* g_cond_new() was deprecated in glib 2.31 but we still need to support it
> */
> #if GLIB_CHECK_VERSION(2, 31, 0)
> @@ -86,6 +90,34 @@ typedef struct {
> static void read_from_buffer(unsigned int idx, void *dataptr, size_t size);
> static unsigned int write_to_buffer(unsigned int idx, void *dataptr, size_t
> size);
>
> +/* Hide changes in glib mutex APIs */
> +static void lock_trace_lock(void)
> +{
> +#if GLIB_CHECK_VERSION(2, 32, 0)
> + g_mutex_lock(&trace_lock);
> +#else
> + g_static_mutex_lock(&trace_lock);
> +#endif
> +}
> +
> +static void unlock_trace_lock(void)
> +{
> +#if GLIB_CHECK_VERSION(2, 32, 0)
> + g_mutex_unlock(&trace_lock);
> +#else
> + g_static_mutex_unlock(&trace_lock);
> +#endif
> +}
> +
> +static GMutex *get_trace_lock_mutex(void)
> +{
> +#if GLIB_CHECK_VERSION(2, 32, 0)
> + return &trace_lock;
> +#else
> + return g_static_mutex_get_mutex(&trace_lock);
> +#endif
> +}
I'd group mutex definition above with all the functions accessing it,
and also make the functions inline.
Well, to my taste, this is a good example where #define is better than
an inline function. Compare the above with:
diff --git a/trace/simple.c b/trace/simple.c
index 1e3f691..2e55ac1 100644
--- a/trace/simple.c
+++ b/trace/simple.c
@@ -39,7 +39,17 @@
* Trace records are written out by a dedicated thread. The thread waits for
* records to become available, writes them out, and then waits again.
*/
+#if GLIB_CHECK_VERSION(2, 32, 0)
+static GMutex trace_lock;
+#define lock_trace_lock() g_mutex_lock(&trace_lock)
+#define unlock_trace_lock() g_mutex_unlock(&trace_lock)
+#define get_trace_lock_mutex() (&trace_lock)
+#else
static GStaticMutex trace_lock = G_STATIC_MUTEX_INIT;
+#define lock_trace_lock() g_static_mutex_lock(&trace_lock)
+#define unlock_trace_lock() g_static_mutex_unlock(&trace_lock)
+#define get_trace_lock_mutex() g_static_mutex_get_mutex(&trace_lock)
+#endif
/* g_cond_new() was deprecated in glib 2.31 but we still need to support it */
#if GLIB_CHECK_VERSION(2, 31, 0)
(#defines here and elsewhere has added bonus - when debugging, debugger
does not step into the inline functions, -- such stepping is quite annoying).
But somehow many developers prefer inline functions (sometimes it is better
indeed, especially in a commonly used header files, and when the functions
has complex or many parameters; in this case we have much simpler situation.
For fun, this #ifdeffery is 5 times larger than the actual users of the
functions being defined :)
Thanks,
/mjt
