Wireshark-bugs: [Wireshark-bugs] [Bug 6416] Improved CIP and ENIP dissectors
Date: Tue, 25 Oct 2011 19:00:43 -0700 (PDT)
https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=6416

--- Comment #15 from Jeff Morriss <jeff.morriss.ws@xxxxxxxxx> 2011-10-25 19:00:42 PDT ---
Hmmm, unfortunately the new patch doesn't apply cleanly:

~~~
(Stripping trailing CRs from patch.)
patching file epan/dissectors/packet-cip.c
Hunk #1 succeeded at 14 with fuzz 1.
Hunk #11 FAILED at 2495.
Hunk #13 FAILED at 2681.
Hunk #20 FAILED at 3095.
Hunk #21 FAILED at 3104.
Hunk #38 FAILED at 3982.
Hunk #39 FAILED at 3997.
Hunk #50 FAILED at 4372.
Hunk #52 FAILED at 4460.
8 out of 63 hunks FAILED -- saving rejects to file
epan/dissectors/packet-cip.c.rej
(Stripping trailing CRs from patch.)
patching file epan/dissectors/packet-cip.h
(Stripping trailing CRs from patch.)
patching file epan/dissectors/packet-cipmotion.c
(Stripping trailing CRs from patch.)
patching file epan/dissectors/packet-enip.c
Hunk #1 FAILED at 10.
1 out of 20 hunks FAILED -- saving rejects to file
epan/dissectors/packet-enip.c.rej
(Stripping trailing CRs from patch.)
patching file epan/dissectors/packet-enip.h
~~~

A couple other comments:

1) Shouldn't the tvb_length_remaining() calls be
tvb_reported_length_remaining()?  At least the ones checking for how much data
is left before raising an expert info--since having a capture cut short by a
snaplen is does not a malformed packet make (it makes a truncated packet). 
Another related question is whether those checks are really necessary: if they
weren't there the dissector would walk off the end of the TVB and throw the
appropriate exception.

2) Double check the encoding values.  For example hf_cip_link_address_string is
used with ENC_LITTLE_ENDIAN; that should probably be ENC_ASCII|ENC_NA.

3) This:

+      expert_add_info_format(pinfo, item, PI_UNDECODED, PI_ERROR, "PROGRAMMING
ERROR: Attribute table set up incorrectly");

Should probably be a DISSECTOR_ASSERT() since it appears to really be a
programming error (not a problem with the packet).  That way our automated fuzz
testing will detect if/when we hit that situation.

4) tvb_get_ephemeral_faked_unicode() has been superseded by
tvb_get_ephemeral_string().

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