[Qemu-devel] [PATCH 4/5] libcacard: replace qemu thread primitives with glib ones

2014-04-29 Thread Michael Tokarev
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

2014-04-29 Thread Christophe Fergeau
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

2014-04-29 Thread Paolo Bonzini

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

2014-04-29 Thread Michael Tokarev
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

2014-04-29 Thread 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.  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

2014-04-29 Thread Michael Tokarev
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

2014-04-29 Thread Michael Tokarev
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