[Qemu-devel] [PATCH v2 0/7] 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.

Another approach which has been implemented earlier by Stefan Hajnoczi
is to wrap deprecated calls into defines which uses the new API.
To my taste this is more intrusive, since it changes all callers
in non-obvious ways.  The approach presented in this series is
more hackish for sure, but is significantly less intrusive to the
callers, allowing using new glib API everywhere.

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.

Changes from first submission:

 - replaced pstrcpy() with memcpy() in libcacard as suggested by
   Paolo Bonzini, and make this change to be in its own patch

 - added a cleanup patch from Stefan Hajnoczi to move g_poll compat
   code from qemu-common.h to glib-compat.h

 - added call to g_error() to g_thread_new() implementation for old
   interface

 - adjusted commit messages

Patches 1/7, 2/7 and 5/7 can be used independently as small cleanup
patches, suitable for -trivial tree.

Thanks,

/mjt

Michael Tokarev (6):
  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 pstrcpy() with memcpy()
  libcacard: replace qemu thread primitives with glib ones
  libcacard: actually use symbols file

Stefan Hajnoczi (1):
  glib: move g_poll() replacement into glib-compat.h

 coroutine-gthread.c        |   37 +++++---------
 include/glib-compat.h      |  118 ++++++++++++++++++++++++++++++++++++++++++++
 include/qemu-common.h      |   12 -----
 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 ++++----
 10 files changed, 221 insertions(+), 149 deletions(-)

-- 
1.7.10.4