http://bugs.wireshark.org/bugzilla/show_bug.cgi?id=2074
------- Comment #2 from jaap.keuter@xxxxxxxxx 2007-12-06 08:00 GMT -------
The code looks good, a few remarks:
1. The files packet-ff.[ch] miss a license statement. If it's not released
under GPL2 it can't be included in the repository.
2. Many items are added like this:
+ Index = tvb_get_ntohl(tvb, offset);
+ proto_tree_add_uint(sub_tree,
+ hf_ff_fms_info_report_req_idx, tvb, offset, 4, Index);
while the preferred method is to use
+ proto_tree_add_item(sub_tree,
+ hf_ff_fms_info_report_req_idx, tvb, offset, 4, FALSE);
3. Using tvb_memcpy and then casting it to a structure is not a portable way to
read a header. The layout of the structure in memory can't be guaranteed.
+ tvb_memcpy(tvb, (guint8 *)&Header, offset, sizeof(Header));
+ length = pntohl(&(Header.Length));
Since you know the fields in the protocol, parse them as such.
+ length = tvb_get_ntohl(tvb, 8);
--
Configure bugmail: http://bugs.wireshark.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
