Wireshark-bugs: [Wireshark-bugs] [Bug 5881] Media Independent handover (MIH) protocol dissector

Date: Tue, 21 Jun 2011 09:01:53 -0700 (PDT)

Bill Meier <wmeier@xxxxxxxxxxx> changed:

           What    |Removed                     |Added
             Status|ASSIGNED                    |NEW

--- Comment #5 from Bill Meier <wmeier@xxxxxxxxxxx> 2011-06-21 12:01:51 EDT ---
Some comments:

1. Has this dissector been fuzz-tested ? See doc/README.developer

2. Please include a reference (or references) to the protocol spec 
   (including definitions for the TLV types) in the source file.

3. Re:

   #define MIH_PORT 4551
   /*assumed for now but has to be changed accordingly*/

   I'm guessing that the "assumed" comment may no longer be relevant since:

   TCP/UDP port 4551 is assigned to MIH 
  (according to IANA: http://www.iana.org/assignments/port-numbers).

   If the port, in general, can vary then it should be made
   a preference.

4. While indexing through the TLV's there's no checking if the index 'i'
   is greater than 19 thus I think it's possible (though maybe unlikely) that
   'ett_tlv[i]' can reference past the end of the array.

   More broadly, I'm not sure that using an ett_array is the right way to
   go for handling the display of TLV subtrees.
   In general ett's are meant to allow expansion/collapse of specific
   subtrees in all the frames being displayed.

   The code in this case implements expansion/collapse of the n'th
   TLV in all the frames; IMHO this may not be too useful.

   - Just one ett for all the TLV's: This means that either all or none of
     the TLV's will be expanded.
   - An ett for each TLV type (overkill ?).

   Note that clicking to expand a subtree in a frame at first applies only to 
   that sub-tree in that frame.

   However: when another frame is selected, all the subtrees referencing
   the ett for the originally expanded subtree will be displayed as expanded.

5. len_of_len is assigned but never used.

6. All TLV "values" are being displayed as FT_STRING.
   This is appropriate only if the values are really always strings (which
   appears not to be the case).
   If there's not to be TLV type-dependent value dissection, then I think
   FT_BYTES should be used.

7. Looking at the dissection of the provided capture file I note:

   Frame #1: 
      - TLV 3: Unknown TLV type

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