Wireshark-bugs: [Wireshark-bugs] [Bug 6961] New dissector for the HART/IP protocol

Date: Fri, 16 Mar 2012 14:22:24 -0700 (PDT)
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.