diff mbox

[PULL,2/8] vscclient: use glib thread primitives not qemu

Message ID 1402379781-844-3-git-send-email-pbonzini@redhat.com
State New
Headers show

Commit Message

Paolo Bonzini June 10, 2014, 5:56 a.m. UTC
From: Michael Tokarev <mjt@tls.msk.ru>

Use glib-provided thread primitives in vscclient instead of
qemu ones, and do not use qemu sockets in there (open-code
call to WSAStartup() for windows to initialize things).

This way, vscclient becomes more stand-alone, independent on
qemu internals.

Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
Reviewed-by: Alon Levy <alevy@redhat.com>
Tested-by: Alon Levy <alevy@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 libcacard/vscclient.c | 70 +++++++++++++++++++++++++++------------------------
 1 file changed, 37 insertions(+), 33 deletions(-)

Comments

Ed Maste June 17, 2014, 5:10 p.m. UTC | #1
On 10 June 2014 01:56, Paolo Bonzini <pbonzini@redhat.com> wrote:
> From: Michael Tokarev <mjt@tls.msk.ru>
>
> Use glib-provided thread primitives in vscclient instead of
> qemu ones, and do not use qemu sockets in there (open-code
> call to WSAStartup() for windows to initialize things).
>
> This way, vscclient becomes more stand-alone, independent on
> qemu internals.
>
> Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
> Reviewed-by: Alon Levy <alevy@redhat.com>
> Tested-by: Alon Levy <alevy@redhat.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  libcacard/vscclient.c | 70 +++++++++++++++++++++++++++------------------------
>  1 file changed, 37 insertions(+), 33 deletions(-)
>
> diff --git a/libcacard/vscclient.c b/libcacard/vscclient.c
> index 6693900..ab9b2b8 100644
> --- a/libcacard/vscclient.c
> +++ b/libcacard/vscclient.c
> @@ -12,12 +12,10 @@
>
>  #ifndef _WIN32
>  #include <netdb.h>
> +#define closesocket(x) close(x)
>  #endif
> -#include <glib.h>
>
>  #include "qemu-common.h"
> -#include "qemu/thread.h"
> -#include "qemu/sockets.h"

This change breaks the build on FreeBSD:

libcacard/vscclient.c: In function 'send_msg':
libcacard/vscclient.c:111: warning: implicit declaration of function 'htonl'
libcacard/vscclient.c:111: warning: nested extern declaration of 'htonl'
libcacard/vscclient.c: In function 'on_host_init':
libcacard/vscclient.c:248: warning: implicit declaration of function 'ntohl'
libcacard/vscclient.c:248: warning: nested extern declaration of 'ntohl'
libcacard/vscclient.c: In function 'connect_to_qemu':
libcacard/vscclient.c:601: warning: implicit declaration of function 'socket'
libcacard/vscclient.c:601: warning: nested extern declaration of 'socket'
...
Paolo Bonzini June 17, 2014, 5:15 p.m. UTC | #2
Il 17/06/2014 19:10, Ed Maste ha scritto:
> This change breaks the build on FreeBSD:
>
> libcacard/vscclient.c: In function 'send_msg':
> libcacard/vscclient.c:111: warning: implicit declaration of function 'htonl'
> libcacard/vscclient.c:111: warning: nested extern declaration of 'htonl'
> libcacard/vscclient.c: In function 'on_host_init':
> libcacard/vscclient.c:248: warning: implicit declaration of function 'ntohl'
> libcacard/vscclient.c:248: warning: nested extern declaration of 'ntohl'
> libcacard/vscclient.c: In function 'connect_to_qemu':
> libcacard/vscclient.c:601: warning: implicit declaration of function 'socket'
> libcacard/vscclient.c:601: warning: nested extern declaration of 'socket'

Can you prepare a patch yourself?  I would be guessing the right header 
to include, sorry.

Paolo
Ed Maste June 17, 2014, 7:11 p.m. UTC | #3
On 17 June 2014 13:15, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 17/06/2014 19:10, Ed Maste ha scritto:
>
>> This change breaks the build on FreeBSD:
>>
>> libcacard/vscclient.c: In function 'send_msg':
>> libcacard/vscclient.c:111: warning: implicit declaration of function
>> 'htonl'
>> ...
>
> Can you prepare a patch yourself?  I would be guessing the right header to
> include, sorry.

I can get it to build by either restoring the #include of
qemu/sockets.h, or explicitly #including sys/socket.h and
netinet/in.h.  I'm happy to send a signed-off patch for either change,
but it seems there must be more to the original change that I'm
missing.  What provides the declarations for socket(), htonl(),
AF_INET etc. in the Linux build?
Paolo Bonzini June 18, 2014, 9:25 a.m. UTC | #4
Il 17/06/2014 21:11, Ed Maste ha scritto:
> On 17 June 2014 13:15, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> Il 17/06/2014 19:10, Ed Maste ha scritto:
>>
>>> This change breaks the build on FreeBSD:
>>>
>>> libcacard/vscclient.c: In function 'send_msg':
>>> libcacard/vscclient.c:111: warning: implicit declaration of function
>>> 'htonl'
>>> ...
>>
>> Can you prepare a patch yourself?  I would be guessing the right header to
>> include, sorry.
>
> I can get it to build by either restoring the #include of
> qemu/sockets.h, or explicitly #including sys/socket.h and
> netinet/in.h.

The latter is better, this patch meant to remove most (though not yet 
all) dependencies of libcacard on include/qemu/.

   I'm happy to send a signed-off patch for either change,
> but it seems there must be more to the original change that I'm
> missing.  What provides the declarations for socket(), htonl(),
> AF_INET etc. in the Linux build?

netdb.h includes netinet/in.h, and netinet/in.h includes sys/socket.h. 
glibc is not particularly good at avoiding #includes within headers. :(

Paolo
diff mbox

Patch

diff --git a/libcacard/vscclient.c b/libcacard/vscclient.c
index 6693900..ab9b2b8 100644
--- a/libcacard/vscclient.c
+++ b/libcacard/vscclient.c
@@ -12,12 +12,10 @@ 
 
 #ifndef _WIN32
 #include <netdb.h>
+#define closesocket(x) close(x)
 #endif
-#include <glib.h>
 
 #include "qemu-common.h"
-#include "qemu/thread.h"
-#include "qemu/sockets.h"
 
 #include "vscard_common.h"
 
@@ -54,7 +52,7 @@  print_usage(void) {
 
 static GIOChannel *channel_socket;
 static GByteArray *socket_to_send;
-static QemuMutex socket_to_send_lock;
+static CompatGMutex socket_to_send_lock;
 static guint socket_tag;
 
 static void
@@ -103,7 +101,7 @@  send_msg(
 ) {
     VSCMsgHeader mhHeader;
 
-    qemu_mutex_lock(&socket_to_send_lock);
+    g_mutex_lock(&socket_to_send_lock);
 
     if (verbose > 10) {
         printf("sending type=%d id=%u, len =%u (0x%x)\n",
@@ -117,18 +115,18 @@  send_msg(
     g_byte_array_append(socket_to_send, (guint8 *)msg, length);
     g_idle_add(socket_prepare_sending, NULL);
 
-    qemu_mutex_unlock(&socket_to_send_lock);
+    g_mutex_unlock(&socket_to_send_lock);
 
     return 0;
 }
 
 static VReader *pending_reader;
-static QemuMutex pending_reader_lock;
-static QemuCond pending_reader_condition;
+static CompatGMutex pending_reader_lock;
+static CompatGCond pending_reader_condition;
 
 #define MAX_ATR_LEN 40
-static void *
-event_thread(void *arg)
+static gpointer
+event_thread(gpointer arg)
 {
     unsigned char atr[MAX_ATR_LEN];
     int atr_len;
@@ -149,20 +147,20 @@  event_thread(void *arg)
             /* ignore events from readers qemu has rejected */
             /* if qemu is still deciding on this reader, wait to see if need to
              * forward this event */
-            qemu_mutex_lock(&pending_reader_lock);
+            g_mutex_lock(&pending_reader_lock);
             if (!pending_reader || (pending_reader != event->reader)) {
                 /* wasn't for a pending reader, this reader has already been
                  * rejected by qemu */
-                qemu_mutex_unlock(&pending_reader_lock);
+                g_mutex_unlock(&pending_reader_lock);
                 vevent_delete(event);
                 continue;
             }
             /* this reader hasn't been told its status from qemu yet, wait for
              * that status */
             while (pending_reader != NULL) {
-                qemu_cond_wait(&pending_reader_condition, &pending_reader_lock);
+                g_cond_wait(&pending_reader_condition, &pending_reader_lock);
             }
-            qemu_mutex_unlock(&pending_reader_lock);
+            g_mutex_unlock(&pending_reader_lock);
             /* now recheck the id */
             reader_id = vreader_get_id(event->reader);
             if (reader_id == VSCARD_UNDEFINED_READER_ID) {
@@ -178,12 +176,12 @@  event_thread(void *arg)
             /* wait until qemu has responded to our first reader insert
              * before we send a second. That way we won't confuse the responses
              * */
-            qemu_mutex_lock(&pending_reader_lock);
+            g_mutex_lock(&pending_reader_lock);
             while (pending_reader != NULL) {
-                qemu_cond_wait(&pending_reader_condition, &pending_reader_lock);
+                g_cond_wait(&pending_reader_condition, &pending_reader_lock);
             }
             pending_reader = vreader_reference(event->reader);
-            qemu_mutex_unlock(&pending_reader_lock);
+            g_mutex_unlock(&pending_reader_lock);
             reader_name = vreader_get_name(event->reader);
             if (verbose > 10) {
                 printf(" READER INSERT: %s\n", reader_name);
@@ -246,7 +244,6 @@  on_host_init(VSCMsgHeader *mhHeader, VSCMsgInit *incoming)
     int num_capabilities =
         1 + ((mhHeader->length - sizeof(VSCMsgInit)) / sizeof(uint32_t));
     int i;
-    QemuThread thread_id;
 
     incoming->version = ntohl(incoming->version);
     if (incoming->version != VSCARD_VERSION) {
@@ -269,7 +266,7 @@  on_host_init(VSCMsgHeader *mhHeader, VSCMsgInit *incoming)
     send_msg(VSC_ReaderRemove, VSCARD_MINIMAL_READER_ID, NULL, 0);
     /* launch the event_thread. This will trigger reader adds for all the
      * existing readers */
-    qemu_thread_create(&thread_id, "vsc/event", event_thread, NULL, 0);
+    g_thread_new("vsc/event", event_thread, NULL);
     return 0;
 }
 
@@ -379,26 +376,26 @@  do_socket_read(GIOChannel *source,
         case VSC_Error:
             error_msg = (VSCMsgError *) pbSendBuffer;
             if (error_msg->code == VSC_SUCCESS) {
-                qemu_mutex_lock(&pending_reader_lock);
+                g_mutex_lock(&pending_reader_lock);
                 if (pending_reader) {
                     vreader_set_id(pending_reader, mhHeader.reader_id);
                     vreader_free(pending_reader);
                     pending_reader = NULL;
-                    qemu_cond_signal(&pending_reader_condition);
+                    g_cond_signal(&pending_reader_condition);
                 }
-                qemu_mutex_unlock(&pending_reader_lock);
+                g_mutex_unlock(&pending_reader_lock);
                 break;
             }
             printf("warning: qemu refused to add reader\n");
             if (error_msg->code == VSC_CANNOT_ADD_MORE_READERS) {
                 /* clear pending reader, qemu can't handle any more */
-                qemu_mutex_lock(&pending_reader_lock);
+                g_mutex_lock(&pending_reader_lock);
                 if (pending_reader) {
                     pending_reader = NULL;
                     /* make sure the event loop doesn't hang */
-                    qemu_cond_signal(&pending_reader_condition);
+                    g_cond_signal(&pending_reader_condition);
                 }
-                qemu_mutex_unlock(&pending_reader_lock);
+                g_mutex_unlock(&pending_reader_lock);
             }
             break;
         case VSC_Init:
@@ -601,7 +598,7 @@  connect_to_qemu(
     struct addrinfo *server;
     int ret, sock;
 
-    sock = qemu_socket(AF_INET, SOCK_STREAM, 0);
+    sock = socket(AF_INET, SOCK_STREAM, 0);
     if (sock < 0) {
         /* Error */
         fprintf(stderr, "Error opening socket!\n");
@@ -654,8 +651,20 @@  main(
     int cert_count = 0;
     int c, sock;
 
-    if (socket_init() != 0)
+#ifdef _WIN32
+    WSADATA Data;
+
+    if (WSAStartup(MAKEWORD(2, 2), &Data) != 0) {
+        c = WSAGetLastError();
+        fprintf(stderr, "WSAStartup: %d\n", c);
         return 1;
+    }
+#endif
+#if !GLIB_CHECK_VERSION(2, 31, 0)
+    if (!g_thread_supported()) {
+         g_thread_init(NULL);
+    }
+#endif
 
     while ((c = getopt(argc, argv, "c:e:pd:")) != -1) {
         switch (c) {
@@ -722,13 +731,8 @@  main(
     }
 
     socket_to_send = g_byte_array_new();
-    qemu_mutex_init(&socket_to_send_lock);
-    qemu_mutex_init(&pending_reader_lock);
-    qemu_cond_init(&pending_reader_condition);
-
     vcard_emul_init(command_line_options);
-
-    loop = g_main_loop_new(NULL, true);
+    loop = g_main_loop_new(NULL, TRUE);
 
     printf("> ");
     fflush(stdout);