Wireshark-bugs: [Wireshark-bugs] [Bug 5868] USB-encapsulated AT/Hayes Commands dissector
https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=5868
--- Comment #6 from Chris Maynard <christopher.maynard@xxxxxxxxx> 2011-04-28 12:57:21 PDT ---
Comments on the dissector:
1) check_col() is deprecated
2) Can the command string be displayed in the Info column?
3) Should this just be packet-at.c? I don't recall the original Hayes AT
command set, but there are lots more AT commands than just those now.
4) Both dissect_usb_hayes() returns are within the if (tvb_memeql(...)==0) {}
block, so return (TRUE) is actually dead code and results in a compiler
warning/error. The return (FALSE) belongs outside the block.
5) In proto_reg_handoff_hayes_command(), since there are no preferences, you
don't need to wrap stuff within "if (!initialized) {}". (See basic explanation
in README.developer sample proto_reg_handoff_PROTOABBREV)
6) I have AT/USB captures of commands that don't begin with CRLF; they being
with "AT".
7) In any case, I think the heuristics are too weak. We might strengthen them
to look to see if the payload only contains expected/allowed characters using
ctype.h's isxxx() macros, or strspn() or strcspn() or the like? In the end
though, I wonder if even that could be relied upon and if a "Decode As ..."
might have to be employed as mentioned in bug 4819? If enhancement bug 2931 is
ever implemented, you wouldn't have to perform the "Decode As ..." every time.
IDK, maybe there's a better way?
8) Why declare proto_hayes extern in packet-hayes.h? Why is packet-hayes.h
needed at all?
9) I'm confused about the heur_dissector_add() call. The USB dissector has a
usb.bulk dissector table, but it uses register_dissector_table() to register it
and calls dissector_try_uint(), not dissector_try_heuristic(). Don't you need
to use dissector_add_uint("usb.bulk", IF_CLASS_?, hayes_handle)? In fact,
until I changed it to dissector_add_uint(), I was getting a runtime error at
startup. Did you add heuristic support in packet-usb.c perhaps?
--
Configure bugmail: https://bugs.wireshark.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
