Wireshark · Wireshark-bugs: [Wireshark-bugs] [Bug 2074] new protocol dissector for the protocol FF-HSE

[bugzilla-daemon@xxxxxxxxxxxxx]

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.