[Qemu-devel] [PATCH 0/5] glib thread interface and libcacard cleanups

[Michael Tokarev]

Basically libgthread has been rewritten in glib version 2.31, and old ways
to use thread primitives stopped working while new ways appeared.  The two
interfaces were sufficiently different to warrant large ifdeffery across all
code using it.

Here's a patchset which tries to clean usage of glib thread interface across
this major rewrite.

The main change was that certain primitives - conditionals and mutexes -
were dynamic-only before 2.31 (ie, should be allocated using foo_new() and
freed using foo_free()), while in 2.31 and up, _new()/_free() has been
deprecated, and new primitives, _init()/_clear(), were added.  So before
2.31, we had to declare a pointer call foo_new() to allocate actual object,
and use this pointer when calling all functions which use this object,
while in 2.31+, we have to declare actual object and use its address when
calling functions.

The trick to make this stuff happy for old glib which I used is to re-define
actual type to be a pointer to that type, using #define, like this:

  #define GMutex GMutex*

so every time the code refers to GMutex it actually refers to a pointer to
that object.  Plus wrapper #define and inline functioins which accept such
a pointer and call actual glib function after dereferencing it, like this:

  static inline g_forward_compat_mutex_lock(GMutex **mutex)
  {
    g_mutex_lock(*mutex);
  }
  #undef g_mutex_lock
  #define g_mutex_lock(mutex) g_forward_compat_mutex_lock(mutex)

This way, we use new, 2.31+, glib interface everywhere, but for pre-2.31
glib, this interface is wrapped using old API and by converting the
actual object to a pointer to actual object behind the scenes.

It is hackish, but this allows to write very very clean, new-style, code,
and compile it with old glib.

The only difference with actual new interface is that in new, 2.31+, glib,
those objects, when declared statically, don't need to be initialized and
will just work when passed to their functions.  While for old interface,
actual objects needs to be allocated using g_foo_new().  So I added a
set of functions, g_foo_init_static(), which should be called in the same
place where old code expected to have g_foo_new().  For new interface
those functions evaluates to nothing, but for old interface they call
the allocation routine.

It is not the same as g_foo_init(), -- I wanted to distinguish this
_static() method from regular init() (tho any of those can be used),
because it is easy this way to find places in the code which can
benefit from cleanups later when we'll drop support for glib < 2.31.

The series consists of 5 patches:

- do not call g_thread_init() for glib >= 2.31

 This is a cleanup patch, cleaning g_thread_init() call.  In 2.31+,
 threads are always enabled and initialized (and glib can't be built
 without threads), and g_thread_supported() is #defined to be 1.
 So the #ifdeffery didn't make any sense to start with, especially
 printing error message and aborting if !g_thread_supported().

- glib-compat.h: add new thread API emulation on top of pre-2.31 API

 This is the main and largest part.  Introducing described above
 compatibility layer into include/glib-compat.h.

 This patch also cleans up direct usage of GCond and GMutex in the code
 in 2 fles: coroutine-gthread.c and trace/simple.c.  In the latter,
 whole ifdeffery is eliminated this way completely, while in the
 latter, there's one more primitive which received rewrite in the
 same version of glib, -- thread private data, GPrivate and GStaticPrivate,
 which still uses #ifdefs.

 I had to introduce the compat layer together with the changes in usage,
 because else the ifdeffery around usage conflicts with the compat layer.

 In coroutine-gthread.c, I also replaced GStaticMutex (from old glib)
 with regular GMutex.  The thing is that actually, GStaticMutex was
 very similar to what I've done with the actual object vs a pointer to
 object - it works in term of GMutex, but stores just a pointer of it,
 and allocates it on demand dynamically.  Using GMutex directly makes
 it a bit faster.

- vscclient: use glib thread primitives not qemu
- libcacard: replace qemu thread primitives with glib ones

 convert libcacard from qemu thread primitives to glib thread primitives
 using the new compatibility layer.  This way, libcacard becomes stand-alone
 library and does not depend on qemu anymore, so programs using it are
 not required to mess with qemu objects.

 an unrelated-to-glib change which I had to apply to libcacard here was
 to replace pstrcpy() back to strncpy().  The reverse conversion has been
 done in the past, this patch reverts it back to usage of strncpy().

 and we've some tiny OS-compat code added to vscclient.c here.

- libcacard: actually use symbols file

 this is the change which started whole series.  This patch makes export
 list for libcacard.so to be strict, exporting only really necessary
 symbols, omitting internal symbols.  Previously, libcacard used and
 exported many of qemu internals, including thread functions.  Now
 it not only stopped exporting them, but also stopped using them.

The whole thing has been compile-tested with both new and old glib
versions on linux and FreeBSD, and runtime-tested on linux (again,
both old and new versions) with --with-coroutine=gthread.  I didn't
test libcacard much, because I found no testcases for it, but at
least it _appears_ to work.

The diffstat below does not look like a diffstat of a cleanup, because
the patchset adds about 2 times more lines than it removes.  This is
because of large addition to glib-compat.h, plus addition of compat
code to vscclient, to make it independent of qemu.

and a few others.

Thanks,

/mjt

Michael Tokarev (5):
  do not call g_thread_init() for glib >= 2.31
  glib-compat.h: add new thread API emulation on top of pre-2.31 API
  vscclient: use glib thread primitives not qemu
  libcacard: replace qemu thread primitives with glib ones
  libcacard: actually use symbols file

 coroutine-gthread.c        |   37 ++++++----------
 include/glib-compat.h      |  103 ++++++++++++++++++++++++++++++++++++++++++++
 libcacard/Makefile         |   10 +----
 libcacard/event.c          |   25 ++++++-----
 libcacard/vcard_emul_nss.c |    3 +-
 libcacard/vreader.c        |   19 ++++----
 libcacard/vscclient.c      |   75 +++++++++++++++++++-------------
 trace/simple.c             |   50 ++++++---------------
 util/osdep.c               |   21 ++++-----
 9 files changed, 206 insertions(+), 137 deletions(-)

-- 
1.7.10.4