Re: [Qemu-devel] [PATCH 0/5] glib thread interface and libcacard cleanups
On 05/05/2014 07:36 PM, Michael Tokarev wrote: 05.05.2014 18:49, Alon Levy wrote: 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. [...] [] Reviewed-by: Alon Levy al...@redhat.com Tested-by: Alon Levy al...@redhat.com Hmm. Now I'm a bit confused. Which version did you test? :) I posted a v2 patch which splits one of the changes into two (pstrcpy to memcpy conversion in libcacard), added some more (minor) changes (which should not affect libcacard code in any way), and adjusted commit messages. The main code is not affected (or should not be), so Tested-by probably may stay, except of the pstrcpy to memcpy patch (http://patchwork.ozlabs.org/patch/345002/) which may affect libcacard. Here's the v2: http://lists.gnu.org/archive/html/qemu-devel/2014-05/msg00286.html If you tested the git branch which I referred to, that's the v2, not original submission which you're replying to. I've tested and reviewed 7191b2c43eecc52994924245720c534ea1a0dc84 so v2, my bad. 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 Well, this can easily go to -trivial, as I'm planning to send a pull request for it soon anyway. Thank you! /mjt
Re: [Qemu-devel] [PATCH 0/5] glib thread interface and libcacard cleanups
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
Re: [Qemu-devel] [PATCH 0/5] glib thread interface and libcacard cleanups
05.05.2014 18:49, Alon Levy wrote: 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. [...] [] Reviewed-by: Alon Levy al...@redhat.com Tested-by: Alon Levy al...@redhat.com Hmm. Now I'm a bit confused. Which version did you test? :) I posted a v2 patch which splits one of the changes into two (pstrcpy to memcpy conversion in libcacard), added some more (minor) changes (which should not affect libcacard code in any way), and adjusted commit messages. The main code is not affected (or should not be), so Tested-by probably may stay, except of the pstrcpy to memcpy patch (http://patchwork.ozlabs.org/patch/345002/) which may affect libcacard. Here's the v2: http://lists.gnu.org/archive/html/qemu-devel/2014-05/msg00286.html If you tested the git branch which I referred to, that's the v2, not original submission which you're replying to. 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 Well, this can easily go to -trivial, as I'm planning to send a pull request for it soon anyway. Thank you! /mjt
[Qemu-devel] [PATCH 0/5] glib thread interface and libcacard cleanups
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,
