[Qemu-devel] [PATCH] ccid-card-emulated: use EventNotifier
Shut up Coverity's complaint about unchecked fcntl return values,
and especially make the code simpler and more efficient.
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
hw/usb/ccid-card-emulated.c | 29 ++---
1 file changed, 10 insertions(+), 19 deletions(-)
diff --git a/hw/usb/ccid-card-emulated.c b/hw/usb/ccid-card-emulated.c
index 7213c89..aa1c37a 100644
--- a/hw/usb/ccid-card-emulated.c
+++ b/hw/usb/ccid-card-emulated.c
@@ -126,7 +126,7 @@ struct EmulatedState {
QemuMutex vreader_mutex; /* and guest_apdu_list mutex */
QemuMutex handle_apdu_mutex;
QemuCond handle_apdu_cond;
-int pipe[2];
+EventNotifier notifier;
int quit_apdu_thread;
QemuThread apdu_thread_id;
};
@@ -162,9 +162,7 @@ static void emulated_push_event(EmulatedState *card,
EmulEvent *event)
qemu_mutex_lock(card-event_list_mutex);
QSIMPLEQ_INSERT_TAIL((card-event_list), event, entry);
qemu_mutex_unlock(card-event_list_mutex);
-if (write(card-pipe[1], card, 1) != 1) {
-DPRINTF(card, 1, write to pipe failed\n);
-}
+event_notifier_set(card-notifier);
}
static void emulated_push_type(EmulatedState *card, uint32_t type)
@@ -358,16 +356,12 @@ static void *event_thread(void *arg)
return NULL;
}
-static void pipe_read(void *opaque)
+static void card_event_handler(EventNotifier *notifier)
{
-EmulatedState *card = opaque;
+EmulatedState *card = container_of(notifier, EmulatedState, notifier);
EmulEvent *event, *next;
-char dummy;
-int len;
-do {
-len = read(card-pipe[0], dummy, sizeof(dummy));
-} while (len == sizeof(dummy));
+event_notifier_test_and_clear(card-notifier);
qemu_mutex_lock(card-event_list_mutex);
QSIMPLEQ_FOREACH_SAFE(event, card-event_list, entry, next) {
DPRINTF(card, 2, event %s\n,
emul_event_to_string(event-p.gen.type));
@@ -404,16 +398,13 @@ static void pipe_read(void *opaque)
qemu_mutex_unlock(card-event_list_mutex);
}
-static int init_pipe_signaling(EmulatedState *card)
+static int init_event_notifier(EmulatedState *card)
{
-if (pipe(card-pipe) 0) {
-DPRINTF(card, 2, pipe creation failed\n);
+if (event_notifier_init(card-notifier, false) 0) {
+DPRINTF(card, 2, event notifier creation failed\n);
return -1;
}
-fcntl(card-pipe[0], F_SETFL, O_NONBLOCK);
-fcntl(card-pipe[1], F_SETFL, O_NONBLOCK);
-fcntl(card-pipe[0], F_SETOWN, getpid());
-qemu_set_fd_handler(card-pipe[0], pipe_read, NULL, card);
+event_notifier_set_handler(card-notifier, card_event_handler);
return 0;
}
@@ -500,7 +491,7 @@ static int emulated_initfn(CCIDCardState *base)
qemu_cond_init(card-handle_apdu_cond);
card-reader = NULL;
card-quit_apdu_thread = 0;
-if (init_pipe_signaling(card) 0) {
+if (init_event_notifier(card) 0) {
return -1;
}
--
1.9.3
Re: [Qemu-devel] [PATCH] ccid-card-emulated: use EventNotifier
Shut up Coverity's complaint about unchecked fcntl return values,
and especially make the code simpler and more efficient.
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
Reviewed-by: Alon Levy al...@redhat.com
one question below.
---
hw/usb/ccid-card-emulated.c | 29 ++---
1 file changed, 10 insertions(+), 19 deletions(-)
diff --git a/hw/usb/ccid-card-emulated.c b/hw/usb/ccid-card-emulated.c
index 7213c89..aa1c37a 100644
--- a/hw/usb/ccid-card-emulated.c
+++ b/hw/usb/ccid-card-emulated.c
@@ -126,7 +126,7 @@ struct EmulatedState {
QemuMutex vreader_mutex; /* and guest_apdu_list mutex */
QemuMutex handle_apdu_mutex;
QemuCond handle_apdu_cond;
-int pipe[2];
+EventNotifier notifier;
int quit_apdu_thread;
QemuThread apdu_thread_id;
};
@@ -162,9 +162,7 @@ static void emulated_push_event(EmulatedState *card,
EmulEvent *event)
qemu_mutex_lock(card-event_list_mutex);
QSIMPLEQ_INSERT_TAIL((card-event_list), event, entry);
qemu_mutex_unlock(card-event_list_mutex);
-if (write(card-pipe[1], card, 1) != 1) {
-DPRINTF(card, 1, write to pipe failed\n);
-}
+event_notifier_set(card-notifier);
}
static void emulated_push_type(EmulatedState *card, uint32_t type)
@@ -358,16 +356,12 @@ static void *event_thread(void *arg)
return NULL;
}
-static void pipe_read(void *opaque)
+static void card_event_handler(EventNotifier *notifier)
{
-EmulatedState *card = opaque;
+EmulatedState *card = container_of(notifier, EmulatedState, notifier);
EmulEvent *event, *next;
-char dummy;
-int len;
-do {
-len = read(card-pipe[0], dummy, sizeof(dummy));
-} while (len == sizeof(dummy));
+event_notifier_test_and_clear(card-notifier);
Shouldn't the ignored return value be marked somehow?
qemu_mutex_lock(card-event_list_mutex);
QSIMPLEQ_FOREACH_SAFE(event, card-event_list, entry, next) {
DPRINTF(card, 2, event %s\n,
emul_event_to_string(event-p.gen.type));
@@ -404,16 +398,13 @@ static void pipe_read(void *opaque)
qemu_mutex_unlock(card-event_list_mutex);
}
-static int init_pipe_signaling(EmulatedState *card)
+static int init_event_notifier(EmulatedState *card)
{
-if (pipe(card-pipe) 0) {
-DPRINTF(card, 2, pipe creation failed\n);
+if (event_notifier_init(card-notifier, false) 0) {
+DPRINTF(card, 2, event notifier creation failed\n);
return -1;
}
-fcntl(card-pipe[0], F_SETFL, O_NONBLOCK);
-fcntl(card-pipe[1], F_SETFL, O_NONBLOCK);
-fcntl(card-pipe[0], F_SETOWN, getpid());
-qemu_set_fd_handler(card-pipe[0], pipe_read, NULL, card);
+event_notifier_set_handler(card-notifier, card_event_handler);
return 0;
}
@@ -500,7 +491,7 @@ static int emulated_initfn(CCIDCardState *base)
qemu_cond_init(card-handle_apdu_cond);
card-reader = NULL;
card-quit_apdu_thread = 0;
-if (init_pipe_signaling(card) 0) {
+if (init_event_notifier(card) 0) {
return -1;
}
--
1.9.3
Re: [Qemu-devel] [PATCH] ccid-card-emulated: use EventNotifier
Il 23/06/2014 12:38, Alon Levy ha scritto:
{
-EmulatedState *card = opaque;
+EmulatedState *card = container_of(notifier, EmulatedState, notifier);
EmulEvent *event, *next;
-char dummy;
-int len;
-do {
-len = read(card-pipe[0], dummy, sizeof(dummy));
-} while (len == sizeof(dummy));
+event_notifier_test_and_clear(card-notifier);
Shouldn't the ignored return value be marked somehow?
It's a pretty common idiom to test-and-clear the event notifier. Really
hot paths have a while() loop, but for ccid-card-emulated it's simpler
to just go through the main loop again. In fact, it was just what the
pipe-based code used to do. :)
Paolo
