Wireshark · Wireshark-bugs: [Wireshark-bugs] [Bug 6155] Dissector for the USB Integrated Circuit Card Interface Device Class (CC

[bugzilla-daemon@xxxxxxxxxxxxx]

https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=6155

Jeff Morriss <jeff.morriss.ws@xxxxxxxxx> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jeff.morriss.ws@xxxxxxxxx

--- Comment #3 from Jeff Morriss <jeff.morriss.ws@xxxxxxxxx> 2011-10-10 18:44:07 PDT ---
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

-- 
Configure bugmail: https://bugs.wireshark.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching all bug changes.