From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=56112 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Q0ArB-0008IC-SY for qemu-devel@nongnu.org; Thu, 17 Mar 2011 06:55:08 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Q0Aqq-0003gQ-02 for qemu-devel@nongnu.org; Thu, 17 Mar 2011 06:54:45 -0400 Received: from mx1.redhat.com ([209.132.183.28]:42382) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Q0Aqp-0003gE-Lk for qemu-devel@nongnu.org; Thu, 17 Mar 2011 06:54:43 -0400 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id p2HAsgQj006603 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Thu, 17 Mar 2011 06:54:42 -0400 Date: Thu, 17 Mar 2011 12:54:16 +0200 From: Alon Levy Subject: Re: [Qemu-devel] [PATCH 5/7] ccid: add ccid-card-emulated device Message-ID: <20110317105416.GR7413@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> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4D7E370E.6090602@redhat.com> List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jes Sorensen Cc: qemu-devel@nongnu.org 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