Re: [xf86-video-qxl v5] Enable smartcard support for XSpice.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Uri,

+AC_ARG_ENABLE([ccid],
+            [AS_HELP_STRING([--enable-ccid],
+            [Build the spiceccid SmartCard driver (default is no)])],
+            [enable_ccid=$enableval],
+            [enable_ccid=no])
+AC_ARG_WITH(ccid-module-dir,
+            [AS_HELP_STRING([--with-ccid-module-dir=DIR ],
+            [Specify the install path for spiceccid driver (default
is $libdir/pcsc/drivers/serial)])],
+            [ cciddir="$withval" ],
+            [ cciddir="$libdir/pcsc/drivers/serial" ])
+AC_SUBST(cciddir)

ccid requires xspice (rephrase error message at will):

+if test "x$enable_ccid" != "xno" && test "x$enable_xspice" = "xno"; then
+    AC_MSG_ERROR([Building with ccid requires xspice, but xspice is not
enabled])
+fi

Sure.

+static void process_reader_add(smartcard_ccid_t *ccid, VSCMsgHeader
*h, char *data)
+{
+    ccid->state |= STATE_READER_ADDED;

ccid->state &= ~STATE_READER_REMOVED;

Yeah, caught that with Marc-Andre's comments.

+static void process_card_remove(smartcard_ccid_t *ccid, VSCMsgHeader *h)
+{
+    ccid->atr_len = 0;
+    memset(ccid->atr, 0, sizeof(ccid->atr));
+    send_reply(ccid, VSC_SUCCESS);

Trying to understand the flow:
Is there a way (API) to notify pcsc-lite ?
Does it get to know about this in the next IFDHTransmitToICC, where
it tries to send as if nothing happened and receive an error code ?



I'm not aware of any way to notify pcscd through the IFD protocol of a closure. Afaict, it just uses those error codes as a signal for device removal.

As a side note, I have patches for pcsc-lite that I have been using which enable user-mode operation for pcscd, and also allow a 'oneshot' capability - run once and die. I've shared those patches with the upstream project, but they were against an old version. I'm hoping to respin them for the modern version shortly.


+    return(MIN(len, h.length + sizeof(h)));

Nitpick: Note that for sure len >= sizeof(h) + h.length as it's checked
at the beginning of the function,
So it should be safe to return h.length + sizeof(h) (as long as that
check stays in the function).
Possibly the compiler optimization takes care of this.

Yeah, you're right.


+RESPONSECODE IFDHCloseChannel(DWORD Lun)
+{
+    int i;
+
+    for (i = 0; i < MAX_LUNS; i++) {
+        if (luns[i].fd != -1 && luns[i].lun == Lun) {
+            pthread_cancel(luns[i].tid);
+            close(luns[i].fd);
+            luns[i].fd = -1;
+            luns[i].lun = 0;
+            luns[i].atr_len = 0;

luns[i].state &= ~STATE_READER_OPEN;

Sure.

+static int smartcard_read(SpiceCharDeviceInstance *sin, uint8_t *buf,
int len)
+{
+    int rc;
+
+    if (smartcard_sin.fd == -1)
+        return 0;
+

Maybe retry immediately for EINTR:

do {
    rc = read()
} while (! ((rc == -1) && (errno == EINTR)))

and remove "|| errno == EINTR" below.

Can you explain why? I imagine that the EINTR case should be quite rare, and that in those cases, it would return into the regular channel handling, where it will be flagged as readable, and eventually return.

Thanks for the further review; it is greatly appreciated.

Cheers,

Jeremy
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel





[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]