Ethereal-dev: Re: [Ethereal-dev] Reaction of g_assert maybe too hard for buggy dissectors?
Ulf Lamping wrote:
I've added the new exception "FieldError", which should be triggered if
a buggy dissector wanted to add a new field with some buggy parameters.
I've renamed it DissectorError, so it can be used to report arbitrary
failed assertions within dissectors.
I've also added a DISSECTOR_ASSERT() macro in <epan/proto.h>; it takes a
Boolean expression as an argument, and, if it evaluates to "false",
generates a string with "g_strdup_printf()" with the source file and
line number of the assertion, and the expression as a string, and throws
a Dissector Error exception with that string as the message.
The show_exception function will then add to:
info column: [Dissector bug]
packet details: [FieldError "FrameID" - "pn_rt.frame_id" invalid length:
-2 proto.c/2098]
console: FieldError in packet: 700 ("FrameID" - "pn_rt.frame_id" invalid
length: -2 proto.c/2098)
I've made the Info column include the full message, and made it, the
packet details, and the console message include pinfo->current_proto, as
an exception thrown in an epan/proto.c routine will have the file and
line being something in epan/proto.c rather than the dissector that
called that routine.
We could add an additional macro to allow the exception to be thrown
with a user-defined string rather than an assertion failure string, so
that we could, for example, show details of the sort reported for
invalid field lengths. That shouldn't necessarily be required, though -
there might be some assertions where the failed expression would include
enough information to identify the bug, and, in those cases, just
replacing "g_assert()" with "DISSECTOR_ASSERT()" should suffice.
I'll look at adding such a macro.
I don't think that putting up a dialog box each time this appears is a
good idea, as this could result in a lot of boxes going up which is not
a desired behaviour.
Putting one up for the *first* error within the process might be useful,
however; it should tell the user to report the full problem, complete
with the message, to ethereal-dev@xxxxxxxxxxxx (and ask them not to send
it as a screenshot, especially not when embedded in a Word document or a
PowerPoint slide show - or a Pages document or a Keynote slide show or
an OOO.org document or slide show or...).
If that is the mechanism that everybody can live with, the next step to
do is to identify all the places where the g_assert calls should be
replaced (e.g. in proto.c).
We should probably do so in *all* code in the dissector code path, both
in dissectors and in code called from dissectors (but not from outside
dissectors).
