Jeff Morriss wrote:
Hi list,
Bug 1511 replaced a g_assert() by a DISSECTOR_ASSERT() to avoid exiting
on a bad packet, but that will show up as a "dissector bug" when really
the problem is in the packet.
Well:
You should never use g_assert() or exit() or something like that in any
dissector code.
You should never call DISSECTOR_ASSERT() for bad packet data.
DISSECTOR_ASSERT() should only be used to detect bad dissector code
(e.g. causing an endless recursion).
Please read the description for ANSI-C assert() call to get an idea why :-)
Any objections to, say, DISSECTOR_ASSERT_MALFORMED_PACKET which would
throw a BoundsError for use in this kind of situations? (No, it's not
really a bounds error but the effect is the desired one: the user gets
shown "malformed packet".) I have, in the past, also wanted a simple
assertion like that when a dissector finds illogical stuff in the packet.
Yes, I think something like this would be useful.
In the past I've did a workaround for stuff like that, using tvb to get
1000000 bytes or so which are really not in the packet, and therefore I
get a BoundsError. But that's actually a workaround.
However, I don't think that DISSECTOR_ASSERT_MALFORMED_PACKET is a good
name for it. That translates to: "be sure that the packet is malformed"
- sounds strange to me.
Ideas for a better name:
- DISSECTOR_VERIFY_PACKET_DATA
- DISSECTOR_VERIFY_DATA (I prefer this name, as it's shorter and still
makes it clear what it does)
Regards, ULFL