Wireshark-bugs: [Wireshark-bugs] [Bug 5753] New dissector for the openSAFETY protocol
https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=5753
--- Comment #4 from Guy Harris <guy@xxxxxxxxxxxx> 2011-03-19 18:37:26 PDT ---
Some problems:
1) The indentation is a mix of tabs and spaces; note that tab stops are
*not* guaranteed to be every 4 spaces, so using spaces rather than tabs might
be the right thing to do.
2) In dissect_opensafety_message(), the chain of if statements setting the
"type" variable doesn't set "type" if none of the 4 tests succeed - either the
dissector has to give up and report a bad type field or it has to set "type" to
something in that case.
3) In the SERCOS III-on-Ethernet frames in the SERCOS III capture, without
this dissector, it correctly dissects the Ethernet header and the SERCOS III
header following it, but, with this dissector, it treats the first byte of the
destination MAC address as the first byte of the SERCOS III header. To fix
this:
do
heur_dissector_add("epl", dissect_heur_opensafety_epl,
proto_opensafety);
/* For SercosIII we have to register as a heuristic dissector, as
SercosIII
* is implemented as a plugin, and therefore the heuristic dissector
is not
* added by the time this method is being called
*/
heur_dissector_add("sercosiii", dissect_heur_opensafety_siii,
proto_opensafety);
in proto_reg_handoff_opensafety() (*N*O*T* in
proto_register_opensafety()!), and remove from dissect_heur_opensafety_siii()
the checks for the Ethernet type for SERCOS III;
4) The "We have an IP packet" comments aren't correct - what you actually
have is a SERCOS III packet, whether it's SERCOS III-over-UDP or SERCOS
III-over-Ethernet;
5) You g_malloc private data, but never free it - you should probably use
ep_alloc() instead (ep_allocated data gets freed when the dissection is
complete).
--
Configure bugmail: https://bugs.wireshark.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
