Wireshark mailing list archives

Re: [Wireshark-commits] rev 32519: /trunk/epan/dissectors/ /trunk/epan/dissectors/: packet-rsvp.c


From: Jaap Keuter <jaap.keuter () xs4all nl>
Date: Wed, 28 Apr 2010 23:45:07 +0200

Hi,

Why the strong position that "it must be a dissector bug"?
 From a comment? What if that comment was misguided, and the author just didn't 
know how to handle these cases/wasn't aware they exist? Even the original text 
added to the tree says "Unknown".
If this really is a problem then the dissector should be fixed, or rolled back.
Running randpkt into the ground like this isn't the right way forward.

I vote for rolling back rev 32519[1] and not backport it to the stable branch.

[1] 
http://anonsvn.wireshark.org/viewvc/trunk/epan/dissectors/packet-rsvp.c?r1=32519&r2=32518&pathrev=32519

Thanks,
Jaap

On 04/28/2010 10:51 PM, Guy Harris wrote:

On Apr 28, 2010, at 1:32 PM, Jeff Morriss wrote:

guy () wireshark org wrote:
http://anonsvn.wireshark.org/viewvc/viewvc.cgi?view=rev&revision=32519

User: guy
Date: 2010/04/19 04:38 PM

Log:
If that should truly "never happen", use DISSECTOR_ASSERT_NOT_REACHED()
so it's more clearly marked as a dissector bug.

(It apparently *does* happen - see bug 4698.)

This has the randpkt test failing on the buildbot.

...which means that the RSVP dissector has, and had even before that checkin, a bug, in that something that, 
according to a comment in the code, "should never happen" can, in fact, happen with a bogus packet; this just makes 
the bug more obvious.

Should it really be backported to 1.2.8?

Clearly marking something that "should never happen" but does happen as a dissector bug in the dissection is better 
than just putting a blob of

      Unknown session type

into the protocol tree, so, yes, I'd backport it.

Or should the randpkt test accept dissector bugs as OK (like the fuzz
testing)?

The fuzz testing accepts dissector bug reports as OK?  That seems like an error to me.

___________________________________________________________________________
Sent via:    Wireshark-dev mailing list <wireshark-dev () wireshark org>
Archives:    http://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
             mailto:wireshark-dev-request () wireshark org?subject=unsubscribe


Current thread: