https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=5881
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.
Possibilities:
- 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.
