Keeping the author of the patch in CC
On Fri, Oct 9, 2015 at 4:34 PM, Christophe Fergeau <cfergeau@xxxxxxxxxx> wrote:
> On Thu, Oct 08, 2015 at 05:40:33PM +0200, marcandre.lureau@xxxxxxxxxx wrote:
>> From: Jeremy White <jwhite@xxxxxxxxxxxxxxx>
>>
>> This uses libpcsclite to provide direct communication with a smartcard.
>>
>> Signed-off-by: Jeremy White <jwhite@xxxxxxxxxxxxxxx>
>> Reviewed-by: Marc-André Lureau <marcandre.lureau@xxxxxxxxxx>
>> ---
>> Makefile.am | 9 +-
>> configure.ac | 1 +
>> src/capcsc.c | 505 ++++++++++++++++++++++++++++++++++++++++++++++++++
>> src/capcsc.h | 18 ++
>> src/card_7816.c | 2 +-
>> src/card_7816.h | 4 +-
>> src/libcacard.syms | 2 +
>> src/vcard.c | 2 +-
>> src/vcard.h | 2 +-
>> src/vcard_emul_nss.c | 11 +-
>> src/vcard_emul_type.c | 1 +
>> 11 files changed, 549 insertions(+), 8 deletions(-)
>> create mode 100644 src/capcsc.c
>> create mode 100644 src/capcsc.h
>>
>> diff --git a/Makefile.am b/Makefile.am
>> index 7939079..b1e257f 100644
>> --- a/Makefile.am
>> +++ b/Makefile.am
>> @@ -15,9 +15,14 @@ libcacard_la_SOURCES = \
>> src/vreader.c \
>> $(NULL)
>>
>> +if ENABLE_PCSC
>> +libcacard_la_SOURCES += src/capcsc.c
>> +endif
>> +
>> libcacard_includedir = $(includedir)/cacard
>> libcacard_include_HEADERS = \
>> src/cac.h \
>> + src/capcsc.h \
>> src/card_7816.h \
>> src/card_7816t.h \
>> src/eventt.h \
>> @@ -32,7 +37,7 @@ libcacard_include_HEADERS = \
>> src/vscard_common.h \
>> $(NULL)
>>
>> -libcacard_la_LIBADD = $(CACARD_LIBS)
>> +libcacard_la_LIBADD = $(CACARD_LIBS) $(PCSC_LIBS)
>> libcacard_la_LDFLAGS = \
>> -export-symbols $(srcdir)/src/libcacard.syms \
>> -no-undefined \
>> @@ -56,7 +61,7 @@ if OS_WIN32
>> vscclient_CFLAGS += -D__USE_MINGW_ANSI_STDIO=1
>> endif
>>
>> -AM_CPPFLAGS = $(CACARD_CFLAGS) $(WARN_CFLAGS)
>> +AM_CPPFLAGS = $(CACARD_CFLAGS) $(PCSC_CFLAGS) $(WARN_CFLAGS)
>> EXTRA_DIST = \
>> NEWS \
>> README.md \
>> diff --git a/configure.ac b/configure.ac
>> index b878526..6a0a73d 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -49,6 +49,7 @@ if test "x$enable_pcsc" != "xno"; then
>> fi
>> if test "x$have_pcsc" = "xyes"; then
>> enable_pcsc=yes
>> + AC_DEFINE([ENABLE_PCSC], 1, [pcsc support])
>> fi
>> fi
>
> This could go in the patch adding the configure check.
ok
>
>> AM_CONDITIONAL(ENABLE_PCSC, test "x$enable_pcsc" = "xyes")
>> diff --git a/src/capcsc.c b/src/capcsc.c
>> new file mode 100644
>> index 0000000..3b68ee3
>> --- /dev/null
>> +++ b/src/capcsc.c
>> @@ -0,0 +1,505 @@
>> +/*
>> + * Supply a vreader using the PC/SC interface.
>> + *
>> + * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
>> + * See the COPYING.LIB file in the top-level directory.
>> + */
>> +
>> +#include "glib-compat.h"
>> +#include <string.h>
>> +#include <stdio.h>
>> +
>> +#include "vcard.h"
>> +#include "card_7816.h"
>> +#include "capcsc.h"
>> +#include "vreader.h"
>> +#include "vevent.h"
>> +
>> +#include <PCSC/wintypes.h>
>> +#include <PCSC/winscard.h>
>> +
>> +
>> +typedef struct _PCSCContext PCSCContext;
>> +
>> +typedef struct {
>> + PCSCContext *context;
>> + int index;
>> + char *name;
>> + DWORD protocol;
>> + DWORD state;
>> + SCARDHANDLE card;
>> + BYTE atr[MAX_ATR_SIZE];
>> + DWORD atrlen;
>> + int card_connected;
>> + unsigned long request_count;
>> +} SCardReader;
>> +
>> +typedef struct _PCSCContext {
>> + SCARDCONTEXT context;
>> + SCardReader readers[CAPCSC_MAX_READERS];
>> + int reader_count;
>> + int readers_changed;
>> + GThread *thread;
>> + CompatGMutex lock;
>> +} PCSCContext;
>> +
>> +
>> +static void delete_reader(PCSCContext *pc, int i)
>> +{
>> + SCardReader *r = &pc->readers[i];
>> + g_free(r->name);
>> + r->name = NULL;
>> +
>> + if (i < (pc->reader_count - 1)) {
>> + int rem = pc->reader_count - i - 1;
>> + memmove(&pc->readers[i], &pc->readers[i + 1],
>> + sizeof(SCardReader) * rem);
>> + }
>
> Since libcacard is using glib, I think GArray/GPtrArray might be usable
> to avoid manual memory juggling. There are few places where memory is
> moved around though, so I'm fine if this stays this way.
>
>
>> +
>> + pc->reader_count--;
>> +}
>> +
>> +static void delete_reader_cb(VReaderEmul *ve)
>> +{
>> + SCardReader *r = (SCardReader *) ve;
>> +
>> + g_mutex_lock(&r->context->lock);
>> + delete_reader(r->context, r->index);
>> + g_mutex_unlock(&r->context->lock);
>> +}
>> +
>> +static int new_reader(PCSCContext *pc, const char *name, DWORD state)
>> +{
>> + SCardReader *r;
>> + VReader *vreader;
>> +
>> + if (pc->reader_count >= CAPCSC_MAX_READERS - 1) {
>> + return 1;
>> + }
>> +
>> + r = &pc->readers[pc->reader_count];
>> + memset(r, 0, sizeof(*r));
>> + r->index = pc->reader_count++;
>> + r->context = pc;
>> + r->name = g_strdup(name);
>> +
>> + vreader = vreader_new(name, (VReaderEmul *) r, delete_reader_cb);
>> + vreader_add_reader(vreader);
>> + vreader_free(vreader);
>> +
>> + return 0;
>> +}
>> +
>> +static int find_reader(PCSCContext *pc, const char *name)
>> +{
>> + int i;
>> + for (i = 0; i < pc->reader_count; i++)
>> + if (strcmp(pc->readers[i].name, name) == 0) {
>> + return i;
>> + }
>> +
>> + return -1;
>> +}
>> +
>> +
>> +static int scan_for_readers(PCSCContext *pc)
>> +{
>> + LONG rc;
>> +
>> + int i;
>> + char buf[8192];
>> + DWORD buflen = sizeof(buf);
>> +
>> + char *p;
>> + int matches[CAPCSC_MAX_READERS];
>> +
>> + g_mutex_lock(&pc->lock);
>> +
>> + for (i = 0; i < CAPCSC_MAX_READERS; i++) {
>> + matches[i] = 0;
>> + }
>
> Why not int matches[..] = { 0, }; or a memset?
make sense
>
>> * destructor for VCardResponse.
>> diff --git a/src/libcacard.syms b/src/libcacard.syms
>> index 2f9d423..1b78f8d 100644
>> --- a/src/libcacard.syms
>> +++ b/src/libcacard.syms
>> @@ -1,4 +1,5 @@
>> cac_card_init
>> +capcsc_init
>> vcard_add_applet
>> vcard_apdu_delete
>> vcard_apdu_new
>
> I don't think you need to export capcsc_init as it's called by
> vcard_emul_init() when needed?
>
>> @@ -40,6 +41,7 @@ vcard_response_new
>> vcard_response_new_bytes
>> vcard_response_new_data
>> vcard_response_new_status_bytes
>> +vcard_response_set_status_bytes
>> vcard_select_applet
>> vcard_set_applet_private
>> vcard_set_atr_func
>> diff --git a/src/vcard.c b/src/vcard.c
>> index 667f30a..2edf1d0 100644
>> --- a/src/vcard.c
>> +++ b/src/vcard.c
>> @@ -97,7 +97,7 @@ vcard_reset(VCard *card, VCardPower power)
>> VCardApplet *
>> vcard_new_applet(VCardProcessAPDU applet_process_function,
>> VCardResetApplet applet_reset_function,
>> - unsigned char *aid, int aid_len)
>> + const unsigned char *aid, int aid_len)
>> {
>> VCardApplet *applet;
>>
>> diff --git a/src/vcard.h b/src/vcard.h
>> index 16a23b5..1364dfb 100644
>> --- a/src/vcard.h
>> +++ b/src/vcard.h
>> @@ -30,7 +30,7 @@ void vcard_reset(VCard *card, VCardPower power);
>> */
>> VCardApplet *vcard_new_applet(VCardProcessAPDU applet_process_function,
>> VCardResetApplet applet_reset_function,
>> - unsigned char *aid, int aid_len);
>> + const unsigned char *aid, int aid_len);
>>
>> /*
>> * destructor for a VCardApplet
>> diff --git a/src/vcard_emul_nss.c b/src/vcard_emul_nss.c
>> index 7ffa0b4..d046ea9 100644
>> --- a/src/vcard_emul_nss.c
>> +++ b/src/vcard_emul_nss.c
>> @@ -9,6 +9,7 @@
>> * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
>> * See the COPYING file in the top-level directory.
>> */
>> +#include "config.h"
>>
>> /*
>> * NSS headers
>> @@ -1253,7 +1254,12 @@ vcard_emul_options(const char *args)
>> opts->hw_card_type = VCARD_EMUL_CAC;
>> opts->use_hw = PR_TRUE;
>> args = find_blank(args + 7);
>> - /* nssemul */
>> +#if defined(ENABLE_PCSC)
>> + } else if (strncmp(args, "passthru", 8) == 0) {
>> + opts->hw_card_type = VCARD_EMUL_PASSTHRU;
>> + opts->use_hw = PR_TRUE;
>> + args = find_blank(args + 8);
>> +#endif
>> } else {
>> fprintf(stderr, "Error: Unknown smartcard specification.\n");
>> return NULL;
>> @@ -1273,6 +1279,9 @@ vcard_emul_usage(void)
>> " hw_type={card_type_to_emulate} (default CAC)\n"
>> " hw_param={param_for_card} (default \"\")\n"
>> " nssemul (alias for use_hw=yes, hw_type=CAC)\n"
>> +#if defined(ENABLE_PCSC)
>> +" passthru (alias for use_hw=yes, hw_type=PASSTHRU)\n"
>> +#endif
>> " soft=({slot_name},{vreader_name},{card_type_to_emulate},{params_for_card},\n"
>> " {cert1},{cert2},{cert3} (default none)\n"
>> "\n"
>
> This is the passthru alias mentioned in another commit log.
>
>> diff --git a/src/vcard_emul_type.c b/src/vcard_emul_type.c
>> index 04e8d99..385e121 100644
>> --- a/src/vcard_emul_type.c
>> +++ b/src/vcard_emul_type.c
>> @@ -7,6 +7,7 @@
>> * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
>> * See the COPYING file in the top-level directory.
>> */
>> +#include "config.h"
>>
>> #include <strings.h>
>> #include "vcardt.h"
>
> Is this hunk required as part of this commit?
for #if defined(ENABLE_PCSC)
>
> Christophe
>
> _______________________________________________
> Spice-devel mailing list
> Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
>
--
Marc-André Lureau
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel