[Qemu-devel] [PATCH 4/5] libcacard: replace qemu thread primitives with glib ones
Replace QemuMutex with GMutex and QemuCond with GCond (with corresponding function changes), to make libcacard independent of qemu internal functions. Also replace single instance pstrcpy() in vcard_emul_nss.c to strncpy(). This reverts commit 2e679780ae86c6ca8. After this step, none of libcacard internals use any qemu-provided symbols. Maybe it's a good idea to stop including qemu-common.h internally too. Signed-off-by: Michael Tokarev m...@tls.msk.ru --- libcacard/Makefile |8 +--- libcacard/event.c | 25 - libcacard/vcard_emul_nss.c |3 ++- libcacard/vreader.c| 19 +-- 4 files changed, 24 insertions(+), 31 deletions(-) diff --git a/libcacard/Makefile b/libcacard/Makefile index 6b06448..89a5942 100644 --- a/libcacard/Makefile +++ b/libcacard/Makefile @@ -3,13 +3,7 @@ libcacard_includedir=$(includedir)/cacard TOOLS += vscclient$(EXESUF) # objects linked into a shared library, built with libtool with -fPIC if required -libcacard-obj-y = $(stub-obj-y) $(libcacard-y) -libcacard-obj-y += util/osdep.o util/cutils.o util/qemu-timer-common.o -libcacard-obj-y += util/error.o util/qemu-error.o -libcacard-obj-$(CONFIG_WIN32) += util/oslib-win32.o util/qemu-thread-win32.o -libcacard-obj-$(CONFIG_POSIX) += util/oslib-posix.o util/qemu-thread-posix.o -libcacard-obj-y += $(filter trace/%, $(util-obj-y)) - +libcacard-obj-y = $(libcacard-y) libcacard-lobj-y=$(patsubst %.o,%.lo,$(libcacard-obj-y)) # libtool will build the .o files, too diff --git a/libcacard/event.c b/libcacard/event.c index 2d7500f..be11596 100644 --- a/libcacard/event.c +++ b/libcacard/event.c @@ -6,7 +6,6 @@ */ #include qemu-common.h -#include qemu/thread.h #include vcard.h #include vreader.h @@ -43,13 +42,13 @@ vevent_delete(VEvent *vevent) static VEvent *vevent_queue_head; static VEvent *vevent_queue_tail; -static QemuMutex vevent_queue_lock; -static QemuCond vevent_queue_condition; +static GMutex vevent_queue_lock; +static GCond vevent_queue_condition; void vevent_queue_init(void) { -qemu_mutex_init(vevent_queue_lock); -qemu_cond_init(vevent_queue_condition); +g_mutex_init_static(vevent_queue_lock); +g_cond_init_static(vevent_queue_condition); vevent_queue_head = vevent_queue_tail = NULL; } @@ -57,7 +56,7 @@ void vevent_queue_vevent(VEvent *vevent) { vevent-next = NULL; -qemu_mutex_lock(vevent_queue_lock); +g_mutex_lock(vevent_queue_lock); if (vevent_queue_head) { assert(vevent_queue_tail); vevent_queue_tail-next = vevent; @@ -65,8 +64,8 @@ vevent_queue_vevent(VEvent *vevent) vevent_queue_head = vevent; } vevent_queue_tail = vevent; -qemu_cond_signal(vevent_queue_condition); -qemu_mutex_unlock(vevent_queue_lock); +g_cond_signal(vevent_queue_condition); +g_mutex_unlock(vevent_queue_lock); } /* must have lock */ @@ -86,11 +85,11 @@ VEvent *vevent_wait_next_vevent(void) { VEvent *vevent; -qemu_mutex_lock(vevent_queue_lock); +g_mutex_lock(vevent_queue_lock); while ((vevent = vevent_dequeue_vevent()) == NULL) { -qemu_cond_wait(vevent_queue_condition, vevent_queue_lock); +g_cond_wait(vevent_queue_condition, vevent_queue_lock); } -qemu_mutex_unlock(vevent_queue_lock); +g_mutex_unlock(vevent_queue_lock); return vevent; } @@ -98,9 +97,9 @@ VEvent *vevent_get_next_vevent(void) { VEvent *vevent; -qemu_mutex_lock(vevent_queue_lock); +g_mutex_lock(vevent_queue_lock); vevent = vevent_dequeue_vevent(); -qemu_mutex_unlock(vevent_queue_lock); +g_mutex_unlock(vevent_queue_lock); return vevent; } diff --git a/libcacard/vcard_emul_nss.c b/libcacard/vcard_emul_nss.c index ee2dfae..0892462 100644 --- a/libcacard/vcard_emul_nss.c +++ b/libcacard/vcard_emul_nss.c @@ -1162,7 +1162,8 @@ vcard_emul_options(const char *args) NEXT_TOKEN(vname) NEXT_TOKEN(type_params) type_params_length = MIN(type_params_length, sizeof(type_str)-1); -pstrcpy(type_str, type_params_length, type_params); +strncpy(type_str, type_params, type_params_length); +type_str[type_params_length] = 0; type = vcard_emul_type_from_string(type_str); NEXT_TOKEN(type_params) diff --git a/libcacard/vreader.c b/libcacard/vreader.c index 5793d73..91799b4 100644 --- a/libcacard/vreader.c +++ b/libcacard/vreader.c @@ -9,10 +9,8 @@ #undef G_LOG_DOMAIN #endif #define G_LOG_DOMAIN libcacard -#include glib.h #include qemu-common.h -#include qemu/thread.h #include vcard.h #include vcard_emul.h @@ -28,7 +26,7 @@ struct VReaderStruct { VCard *card; char *name; vreader_id_t id; -QemuMutex lock; +GMutex lock; VReaderEmul *reader_private; VReaderEmulFree reader_private_free; }; @@ -97,13 +95,13 @@ apdu_ins_to_string(int ins) static inline void vreader_lock(VReader *reader) {
Re: [Qemu-devel] [PATCH 4/5] libcacard: replace qemu thread primitives with glib ones
Hey, patch looks good On Tue, Apr 29, 2014 at 10:02:27AM +0400, Michael Tokarev wrote: Replace QemuMutex with GMutex and QemuCond with GCond (with corresponding function changes), to make libcacard independent of qemu internal functions. Also replace single instance pstrcpy() in vcard_emul_nss.c to strncpy(). This reverts commit 2e679780ae86c6ca8. An alternative would be to use g_strlcpy which guarantees nul-termination. Christophe
Re: [Qemu-devel] [PATCH 4/5] libcacard: replace qemu thread primitives with glib ones
Il 29/04/2014 10:26, Christophe Fergeau ha scritto: Replace QemuMutex with GMutex and QemuCond with GCond (with corresponding function changes), to make libcacard independent of qemu internal functions. Also replace single instance pstrcpy() in vcard_emul_nss.c to strncpy(). This reverts commit 2e679780ae86c6ca8. An alternative would be to use g_strlcpy which guarantees nul-termination. Yes, that is better. Paolo
Re: [Qemu-devel] [PATCH 4/5] libcacard: replace qemu thread primitives with glib ones
29.04.2014 12:38, Paolo Bonzini wrote: Il 29/04/2014 10:26, Christophe Fergeau ha scritto: Replace QemuMutex with GMutex and QemuCond with GCond (with corresponding function changes), to make libcacard independent of qemu internal functions. Also replace single instance pstrcpy() in vcard_emul_nss.c to strncpy(). This reverts commit 2e679780ae86c6ca8. An alternative would be to use g_strlcpy which guarantees nul-termination. Yes, that is better. Actually in this very place it isn't really important, given we always know the exact length of the source and are able to adjust it to fit into the buffer. With g_strlcat() code becomes a bit more ugly... ;) But mind you, this is the least important change in the whole patchset. We are risking to dig into unimportant details and miss forest for the trees in the result. Thanks, /mjt
Re: [Qemu-devel] [PATCH 4/5] libcacard: replace qemu thread primitives with glib ones
Il 29/04/2014 10:42, Michael Tokarev ha scritto: Also replace single instance pstrcpy() in vcard_emul_nss.c to strncpy(). This reverts commit 2e679780ae86c6ca8. An alternative would be to use g_strlcpy which guarantees nul-termination. Yes, that is better. Actually in this very place it isn't really important, given we always know the exact length of the source and are able to adjust it to fit into the buffer. With g_strlcat() code becomes a bit more ugly... ;) Uh, now I looked at NEXT_TOKEN and g_strlcpy suddenly becomes less palatable. mempcpy would be nice actually, like *mempcpy(dest, src, type_params_length) = 0; but it is not portable and not wrapped by glib. Another good alternative is char *type_str; ... type_str = g_strndup(type_params, type_params_length); type = vcard_emul_type_from_string(type_str); g_free(type_str); Paolo
Re: [Qemu-devel] [PATCH 4/5] libcacard: replace qemu thread primitives with glib ones
29.04.2014 13:11, Paolo Bonzini пишет: Il 29/04/2014 10:42, Michael Tokarev ha scritto: Also replace single instance pstrcpy() in vcard_emul_nss.c to strncpy(). This reverts commit 2e679780ae86c6ca8. An alternative would be to use g_strlcpy which guarantees nul-termination. Yes, that is better. Actually in this very place it isn't really important, given we always know the exact length of the source and are able to adjust it to fit into the buffer. With g_strlcat() code becomes a bit more ugly... ;) Uh, now I looked at NEXT_TOKEN and g_strlcpy suddenly becomes less palatable. Yess, that's exactly what I mean. mempcpy would be nice actually, like *mempcpy(dest, src, type_params_length) = 0; but it is not portable and not wrapped by glib. Another good alternative is char *type_str; ... type_str = g_strndup(type_params, type_params_length); type = vcard_emul_type_from_string(type_str); g_free(type_str); Actually it is the best one, -- type_str is g_strndup'ed down the line. I'll do that, in a separate patch before this series. /mjt
Re: [Qemu-devel] [PATCH 4/5] libcacard: replace qemu thread primitives with glib ones
29.04.2014 13:11, Paolo Bonzini wrote: Il 29/04/2014 10:42, Michael Tokarev ha scritto: Also replace single instance pstrcpy() in vcard_emul_nss.c to strncpy(). This reverts commit 2e679780ae86c6ca8. An alternative would be to use g_strlcpy which guarantees nul-termination. Yes, that is better. Actually in this very place it isn't really important, given we always know the exact length of the source and are able to adjust it to fit into the buffer. With g_strlcat() code becomes a bit more ugly... ;) Uh, now I looked at NEXT_TOKEN and g_strlcpy suddenly becomes less palatable. mempcpy would be nice actually, like *mempcpy(dest, src, type_params_length) = 0; I just pushed a new series for this, with this change being in a separate commit. See there: http://git.corpit.ru/?p=qemu.git;a=shortlog;h=refs/heads/libcacard-standalone As promised, I've split this change into a sepatate patch, this: Author: Michael Tokarev m...@tls.msk.ru Date: Tue Apr 29 20:10:48 2014 +0400 libcacard: replace pstrcpy() with memcpy() Commit 2e679780ae86c6ca8 replaced strncpy() with pstrcpy() in one place in libcacard. This is a qemu-specific function, while libcacard is a stand-alone library (or tries to be). But since we know the exact length of the string to copy, and know that it definitely will fit in the destination buffer, use memcpy() instead, and null-terminate the string after that. An alternative is to use g_strlcpy() or strncpy(), but memcpy() is more than adequate in this place. Signed-off-by: Michael Tokarev m...@tls.msk.ru diff --git a/libcacard/vcard_emul_nss.c b/libcacard/vcard_emul_nss.c index ee2dfae..e2b196d 100644 --- a/libcacard/vcard_emul_nss.c +++ b/libcacard/vcard_emul_nss.c @@ -1162,7 +1162,8 @@ vcard_emul_options(const char *args) NEXT_TOKEN(vname) NEXT_TOKEN(type_params) type_params_length = MIN(type_params_length, sizeof(type_str)-1); -pstrcpy(type_str, type_params_length, type_params); +memcpy(type_str, type_params, type_params_length); +type_str[type_params_length] = '\0'; type = vcard_emul_type_from_string(type_str); NEXT_TOKEN(type_params) I also fixed the commit message about vscclient sptted by Christophe. Thanks, /mjt