On 04/29/2014 09:02 AM, Michael Tokarev wrote:
> 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,
>
Reviewed-by: Alon Levy <al...@redhat.com>
Tested-by: Alon Levy <al...@redhat.com>
This would be nice to have too (it has nothing to do with your patchset,
but it would save me a pull request):
commit 2fc95f8bc1912e2de243389d9d102a5a28267f31
Author: Alon Levy <al...@redhat.com>
Date: Mon May 5 17:41:32 2014 +0300
libcacard: remove unnecessary EOL from debug prints
Signed-off-by: Alon Levy <al...@redhat.com>
diff --git a/libcacard/vreader.c b/libcacard/vreader.c
index 91799b4..d1e05af 100644
--- a/libcacard/vreader.c
+++ b/libcacard/vreader.c
@@ -272,12 +272,12 @@ vreader_xfr_bytes(VReader *reader,
response = vcard_make_response(status);
card_status = VCARD_DONE;
} else {
- g_debug("%s: CLS=0x%x,INS=0x%x,P1=0x%x,P2=0x%x,Lc=%d,Le=%d %s\n",
+ g_debug("%s: CLS=0x%x,INS=0x%x,P1=0x%x,P2=0x%x,Lc=%d,Le=%d %s",
__func__, apdu->a_cla, apdu->a_ins, apdu->a_p1, apdu->a_p2,
apdu->a_Lc, apdu->a_Le, apdu_ins_to_string(apdu->a_ins));
card_status = vcard_process_apdu(card, apdu, &response);
if (response) {
- g_debug("%s: status=%d sw1=0x%x sw2=0x%x len=%d (total=%d)\n",
+ g_debug("%s: status=%d sw1=0x%x sw2=0x%x len=%d (total=%d)",
__func__, response->b_status, response->b_sw1,
response->b_sw2, response->b_len, response->b_total_len);
}
> /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(-)
>
