Wireshark-bugs: [Wireshark-bugs] [Bug 6961] New dissector for the HART/IP protocol
https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=6961
--- Comment #2 from Bill Meier <wmeier@xxxxxxxxxxx> 2012-03-16 14:22:23 PDT ---
Some initial comments:
1. Putting 'if (check_col(pinfo->cinfo, COL_INFO))' around almost
all of the dissection will prevent the dissection if someone
should happen to diable the INFO column.
I'd guess that this should just be removed.
2. The proto_tree_add_item() "encoding arg" is no longer
TRUE/FALSE but is ENC_NA, ENC_BIG_ENDIAN, etc.
See doc/README.developer
tools/fix-encoding-arg.pl can be used to change the arg as appropriate
for some of the calls.
I note that others will need to be manually fixed.
3. dissect_packAscii() as written seems to allow 'str' to overflow.
If len is 255, then it appears that 255/3 * 4 chars can be stored
in 'str'.
I realize that packAscii() is never called with len > 24 so the overflow
would never actually occur, but ...
I suggest replacing the 'if (len < 256' with
DISSECTOR_ASSERT(len < 3 * (256/4)) and then allocating 256+1 bytes for the
buffer.
belt & suspenders: add a DISSECTOR_ASSERT(i<256) just before the
str[i++] = ...
4. Please replace the the hf[] "" 'blurb' args with NULL
5. The #include proto.h and #include prefs.h ar not needed.
6. Please remove trailing whitespace from lines.
7. There's some conversation related stuff which is under an 'if (tree)'.
I don't think this is correct.
See README.developer
ISTR that creating new conversations, setting the protocol for the
conversation, etc would normally be done only the first time
a frame is dissected ("first pass").
Take a look at how other dissectors use:
if (!pinfo->fd->flags.visited)
That's all I have time for tight now...
--
Configure bugmail: https://bugs.wireshark.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching all bug changes.
