From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=50678 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Q0AwQ-0004Sv-Nb for qemu-devel@nongnu.org; Thu, 17 Mar 2011 07:00:31 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Q0AwH-00054t-NM for qemu-devel@nongnu.org; Thu, 17 Mar 2011 07:00:22 -0400 Received: from mx1.redhat.com ([209.132.183.28]:19192) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Q0AwH-00054l-7x for qemu-devel@nongnu.org; Thu, 17 Mar 2011 07:00:21 -0400 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id p2HB0J96022423 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Thu, 17 Mar 2011 07:00:20 -0400 Date: Thu, 17 Mar 2011 12:59:53 +0200 From: Alon Levy Subject: Re: [Qemu-devel] [PATCH 5/7] ccid: add ccid-card-emulated device Message-ID: <20110317105953.GS7413@playa.tlv.redhat.com> References: <1298460024-23591-1-git-send-email-alevy@redhat.com> <1298460024-23591-6-git-send-email-alevy@redhat.com> <4D7E370E.6090602@redhat.com> <20110317105416.GR7413@playa.tlv.redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20110317105416.GR7413@playa.tlv.redhat.com> List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jes Sorensen , qemu-devel@nongnu.org On Thu, Mar 17, 2011 at 12:54:16PM +0200, Alon Levy wrote: sorry for the double review of the review, nothing new here. > On Mon, Mar 14, 2011 at 04:41:02PM +0100, Jes Sorensen wrote: > > On 02/23/11 12:20, Alon Levy wrote: > > > diff --git a/hw/ccid-card-emulated.c b/hw/ccid-card-emulated.c > > > new file mode 100644 > > > index 0000000..bd84d45 > > > --- /dev/null > > > +++ b/hw/ccid-card-emulated.c > > > @@ -0,0 +1,599 @@ > > > +/* > > > + * CCID Card Device. Emulated card. > > > + * > > > + * Copyright (c) 2011 Red Hat. > > > + * Written by Alon Levy. > > > + * > > > + * This code is licenced under the GNU LGPL, version 2 or later. > > > + */ > > > + > > > +/* > > > + * It can be used to provide access to the local hardware in a non exclusive > > > + * way, or it can use certificates. It requires the usb-ccid bus. > > > + * > > > + * Usage 1: standard, mirror hardware reader+card: > > > + * qemu .. -usb -device usb-ccid -device ccid-card-emulated > > > + * > > > + * Usage 2: use certificates, no hardware required > > > + * one time: create the certificates: > > > + * for i in 1 2 3; do > > > + * certutil -d /etc/pki/nssdb -x -t "CT,CT,CT" -S -s "CN=user$i" -n user$i > > > + * done > > > + * qemu .. -usb -device usb-ccid \ > > > + * -device ccid-card-emulated,cert1=user1,cert2=user2,cert3=user3 > > > + * > > > + * If you use a non default db for the certificates you can specify it using > > > + * the db parameter. > > > + */ > > > + > > > +#include > > > > qemu-thread.h > ok, fixing. > > > > > > +static const char *emul_event_to_string(uint32_t emul_event) > > > +{ > > > + switch (emul_event) { > > > + case EMUL_READER_INSERT: return "EMUL_READER_INSERT"; > > > + case EMUL_READER_REMOVE: return "EMUL_READER_REMOVE"; > > > + case EMUL_CARD_INSERT: return "EMUL_CARD_INSERT"; > > > + case EMUL_CARD_REMOVE: return "EMUL_CARD_REMOVE"; > > > + case EMUL_GUEST_APDU: return "EMUL_GUEST_APDU"; > > > + case EMUL_RESPONSE_APDU: return "EMUL_RESPONSE_APDU"; > > > + case EMUL_ERROR: return "EMUL_ERROR"; > > > > YUCK! > can we turn down the caps / disgust statements? I understand this > is a personal affront to you somehow? can we settle this at dawn > tommorrow? > > > > > No multi statements on a single line! > > > > > +#define MAX_ATR_SIZE 40 > > > +struct EmulatedState { > > > + CCIDCardState base; > > > + uint8_t debug; > > > + char *backend_str; > > > + uint32_t backend; > > > + char *cert1; > > > + char *cert2; > > > + char *cert3; > > > + char *db; > > > + uint8_t atr[MAX_ATR_SIZE]; > > > + uint8_t atr_length; > > > + QSIMPLEQ_HEAD(event_list, EmulEvent) event_list; > > > + pthread_mutex_t event_list_mutex; > > > + VReader *reader; > > > + QSIMPLEQ_HEAD(guest_apdu_list, EmulEvent) guest_apdu_list; > > > + pthread_mutex_t vreader_mutex; /* and guest_apdu_list mutex */ > > > + pthread_mutex_t handle_apdu_mutex; > > > + pthread_cond_t handle_apdu_cond; > > > + int pipe[2]; > > > + int quit_apdu_thread; > > > + pthread_mutex_t apdu_thread_quit_mutex; > > > + pthread_cond_t apdu_thread_quit_cond; > > > +}; > > > > Bad struct packing and wrong thread types. > will use qemu-thread. that's what you mean by wrong thread types, right? > s/pthread_thread_t/QemuThread/ etc. (Cond, Mutex) > > > > > > +static void emulated_push_type(EmulatedState *card, uint32_t type) > > > +{ > > > + EmulEvent *event = (EmulEvent *)malloc(sizeof(EmulEvent)); > > > > qemu_malloc() > yep, fixing. > > > > > > + assert(event); > > > + event->p.gen.type = type; > > > + emulated_push_event(card, event); > > > +} > > > + > > > +static void emulated_push_error(EmulatedState *card, uint64_t code) > > > +{ > > > + EmulEvent *event = (EmulEvent *)malloc(sizeof(EmulEvent)); > > > > qemu_malloc() > fixing. > > > > > > + assert(event); > > > + event->p.error.type = EMUL_ERROR; > > > + event->p.error.code = code; > > > + emulated_push_event(card, event); > > > +} > > > + > > > +static void emulated_push_data_type(EmulatedState *card, uint32_t type, > > > + const uint8_t *data, uint32_t len) > > > +{ > > > + EmulEvent *event = (EmulEvent *)malloc(sizeof(EmulEvent) + len); > > > > qemu_malloc() > fixing. > > > > > > +static void pipe_read(void *opaque) > > > +{ > > > + EmulatedState *card = opaque; > > > + EmulEvent *event, *next; > > > + char dummy; > > > + int len; > > > + > > > + do { > > > + len = read(card->pipe[0], &dummy, sizeof(dummy)); > > > + } while (len == sizeof(dummy)); > > > > Shouldn't you check error codes here? > yes, my bad. > > > > > Jes >