Message ID | 1363612272-13713-19-git-send-email-alevy@redhat.com |
---|---|
State | New |
Headers | show |
Hi, On Mon, Mar 18, 2013 at 2:11 PM, Alon Levy <alevy@redhat.com> wrote: > + if (atr_protocol_num == 0) { > + DPRINTF(s, D_WARN, "%s: error: unimplemented ATR T0 parameters" > + " setting\n", __func__); > + t0->bmFindexDindex = 0; > + t0->bmTCCKST0 = 0; > + t0->bGuardTimeT0 = 0; > + t0->bWaitingIntegerT0 = 0; > + t0->bClockStop = 0; > + } else { > + if (atr_protocol_num != 1) { > + DPRINTF(s, D_WARN, "%s: error: unsupported ATR protocol %d\n", > + __func__, atr_protocol_num); > + } else { > + DPRINTF(s, D_WARN, "%s: error: unimplemented ATR T1 parameters" > + " setting\n", __func__); > + t1->bmFindexDindex = 0; > + t1->bmTCCKST1 = 0; > + t1->bGuardTimeT1 = 0; > + t1->bWaitingIntegerT1 = 0; > + t1->bClockStop = 0; > + t1->bIFSC = 0; > + t1->bNadValue = 0; > + } > + } Those blocks could be at the same indentation level, or perhaps use a switch? Is it sensible to warn in all cases?
On Fri, Mar 22, 2013 at 03:23:15PM +0100, Marc-André Lureau wrote: > Hi, > > > On Mon, Mar 18, 2013 at 2:11 PM, Alon Levy <alevy@redhat.com> wrote: > > + if (atr_protocol_num == 0) { > > + DPRINTF(s, D_WARN, "%s: error: unimplemented ATR T0 parameters" > > + " setting\n", __func__); > > + t0->bmFindexDindex = 0; > > + t0->bmTCCKST0 = 0; > > + t0->bGuardTimeT0 = 0; > > + t0->bWaitingIntegerT0 = 0; > > + t0->bClockStop = 0; > > + } else { > > + if (atr_protocol_num != 1) { > > + DPRINTF(s, D_WARN, "%s: error: unsupported ATR protocol %d\n", > > + __func__, atr_protocol_num); > > + } else { > > + DPRINTF(s, D_WARN, "%s: error: unimplemented ATR T1 parameters" > > + " setting\n", __func__); > > + t1->bmFindexDindex = 0; > > + t1->bmTCCKST1 = 0; > > + t1->bGuardTimeT1 = 0; > > + t1->bWaitingIntegerT1 = 0; > > + t1->bClockStop = 0; > > + t1->bIFSC = 0; > > + t1->bNadValue = 0; > > + } > > + } > > Those blocks could be at the same indentation level, or perhaps use a switch? A switch is a good idea. > > Is it sensible to warn in all cases? Turning them into TODO comments instead (except for the now default case). > > -- > Marc-André Lureau >
diff --git a/hw/usb/dev-smartcard-reader.c b/hw/usb/dev-smartcard-reader.c index 6894f36..522fefa 100644 --- a/hw/usb/dev-smartcard-reader.c +++ b/hw/usb/dev-smartcard-reader.c @@ -839,14 +839,60 @@ static void ccid_write_data_block_answer(USBCCIDState *s, ccid_write_data_block(s, slot, seq, data, len); } +static uint8_t atr_get_protocol_num(const uint8_t *atr, uint32_t len) +{ + int i; + + if (len < 2 || !(atr[1] & 0x80)) { + /* too short or TD1 not included */ + return 0; /* T=0, default */ + } + i = 1 + !!(atr[1] & 0x10) + !!(atr[1] & 0x20) + !!(atr[1] & 0x40); + i += !!(atr[1] & 0x80); + return atr[i] & 0x0f; +} + static void ccid_write_data_block_atr(USBCCIDState *s, CCID_Header *recv) { const uint8_t *atr = NULL; uint32_t len = 0; + uint8_t atr_protocol_num; + CCID_T0ProtocolDataStructure *t0 = &s->abProtocolDataStructure.t0; + CCID_T1ProtocolDataStructure *t1 = &s->abProtocolDataStructure.t1; if (s->card) { atr = ccid_card_get_atr(s->card, &len); } + atr_protocol_num = atr_get_protocol_num(atr, len); + DPRINTF(s, D_VERBOSE, "%s: atr contains protocol=%d\n", __func__, + atr_protocol_num); + /* set parameters from ATR - see spec page 109 */ + s->bProtocolNum = (atr_protocol_num <= 1 ? atr_protocol_num + : s->bProtocolNum); + if (atr_protocol_num == 0) { + DPRINTF(s, D_WARN, "%s: error: unimplemented ATR T0 parameters" + " setting\n", __func__); + t0->bmFindexDindex = 0; + t0->bmTCCKST0 = 0; + t0->bGuardTimeT0 = 0; + t0->bWaitingIntegerT0 = 0; + t0->bClockStop = 0; + } else { + if (atr_protocol_num != 1) { + DPRINTF(s, D_WARN, "%s: error: unsupported ATR protocol %d\n", + __func__, atr_protocol_num); + } else { + DPRINTF(s, D_WARN, "%s: error: unimplemented ATR T1 parameters" + " setting\n", __func__); + t1->bmFindexDindex = 0; + t1->bmTCCKST1 = 0; + t1->bGuardTimeT1 = 0; + t1->bWaitingIntegerT1 = 0; + t1->bClockStop = 0; + t1->bIFSC = 0; + t1->bNadValue = 0; + } + } ccid_write_data_block(s, recv->bSlot, recv->bSeq, atr, len); }