Message ID | 52A9D7F2.6020301@msgid.tls.msk.ru |
---|---|
State | New |
Headers | show |
On Thu, Dec 12, 2013 at 07:36:18PM +0400, Michael Tokarev wrote: > 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 <stefanha@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 :) Yes, I think you are right. In general I avoid using macros but here it does make things nicer. Stefan
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)