Wireshark · Ethereal-dev: Re: [Ethereal-dev] Re: [Ethereal-cvs] rev 13869: /trunk/epan/dissectors/: packet-wsp.c

[Guy Harris <gharris@xxxxxxxxx>]

Ulf Lamping wrote:

> As this is far better than the former implementation, this still doesn't
> be the right way to handle this.

In that particular case, none of the routines whose return value is 
being checked can return null pointers, so in none of those places is 
the assertion doing a check that bad packet data could cause to fail.

I've removed the DISSECTOR_ASSERT() calls in the WSP dissector - the 
checks you inserted will catch null pointers passed to 
"proto_tree_add_string()".

> If incoming data is wrong, it should be shown as such in the protocol
> tree, but not indicating a dissector bug. Not the dissector is buggy,
> but the data is (important difference).

Yes.

> In a clean dissector implementation, calling DISSECTOR_ASSERT (and
> g_assert) is not required at all. Just don't do: Oh I'm too lazy to
> implement this right now, and this might never happen, so I place a
> DISSECTOR_ASSERT here.

Ideally, it should be implemented; if not, it should at least put 
"Dissection not implemented yet" or something such as that into the 
protocol tree.