Wireshark-bugs: [Wireshark-bugs] [Bug 6155] Dissector for the USB Integrated Circuit Card Interf
Date: Tue, 11 Oct 2011 01:09:35 -0700 (PDT)
https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=6155 --- Comment #4 from Tyson Key <tyson.key@xxxxxxxxx> 2011-10-11 01:09:32 PDT --- (In reply to comment #3) > Looks good, but a few comments: > > 1) Current agreement is that we should not use ENC_NA for FT_*INT8 fields; > instead the protocol's primary endianism should be used. > > 2) Personally I would prefer macros for things like: > {0x61, "PC_to_RDR_SetParameters"}, > [...] > case 0x61 /* PC_to_RDR_SetParameters */: > > 3) Although it's not strictly necessary at the moment, it's good form to set > the column information even when !tree. And this short dissector could be made > even shorter by combining the many col_set_str()'s into one col_add_str(... > val_to_str(cmd, ...) at the top of the function (above the 'if (tree)'). > > 4) checkApis had some complaints about duplicate blurbs: > > Error: the blurb for field "Message Type" ("ccid.bMessageType") matches the > field name in /tmp/packet-usb-ccid.c > Error: the blurb for field "Packet Length" ("ccid.dwLength") matches the field > name in /tmp/packet-usb-ccid.c > Error: the blurb for field "Slot" ("ccid.bSlot") matches the field name in > /tmp/packet-usb-ccid.c > Error: the blurb for field "Sequence" ("ccid.bSeq") matches the field name in > /tmp/packet-usb-ccid.c > Error: the blurb for field "Status" ("ccid.bStatus") matches the field name in > /tmp/packet-usb-ccid.c > Error: the blurb for field "Error" ("ccid.bError") matches the field name in > /tmp/packet-usb-ccid.c > Error: the blurb for field "Chain Parameter" ("ccid.bChainParameter") matches > the field name in /tmp/packet-usb-ccid.c > Error: the blurb for field "Data" ("ccid.abData") matches the field name in > /tmp/packet-usb-ccid.c > Error: the blurb for field "Voltage Level" ("ccid.bPowerSelect") matches the > field name in /tmp/packet-usb-ccid.c > Error: the blurb for field "Clock Status" ("ccid.bClockStatus") matches the > field name in /tmp/packet-usb-ccid.c > Error: the blurb for field "Data Structure Type" ("ccid.bProtocolNum") matches > the field name in /tmp/packet-usb-ccid.c > Error: the blurb for field "Block Wait Time Integer" ("ccid.bBWI") matches the > field name in /tmp/packet-usb-ccid.c > Error: the blurb for field "Level Parameter" ("ccid.wLevelParameter") matches > the field name in /tmp/packet-usb-ccid.c > > I thought about making these changes myself (at least 1, 3, and 4) but ran out > of time Thanks once again for the review, Jeff. I believe that the protocol is mostly *Big Endian (apart from a few fields that I've explicitly specified as Little Endian, according to the specs). Since I'm at university at the moment, I don't have access to my card reader, or to my copies of the appropriate specifications and source code/toolchain. * I think that ENC_NA defaults to that, anyway - so we should be OK with replacing instances of "ENC_NA" with "ENC_BIG_ENDIAN" (or whatever it is). I'll have another look tonight, when I get home, and report back. Tyson. -- Configure bugmail: https://bugs.wireshark.org/bugzilla/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are watching all bug changes.
- Prev by Date: [Wireshark-bugs] [Bug 6161] Dissector for the NXP MiFare Protocol
- Next by Date: [Wireshark-bugs] [Bug 6447] Added support for GPRS NS over ATM AAL5
- Previous by thread: [Wireshark-bugs] [Bug 6155] Dissector for the USB Integrated Circuit Card Interface Device Class (CCID)
- Next by thread: [Wireshark-bugs] [Bug 6155] Dissector for the USB Integrated Circuit Card Interface Device Class (CCID)
- Index(es):
- Get Wireshark
- Download
- Code of Conduct