Wireshark · Ethereal-dev: [Ethereal-dev] TRY_TO_FAKE_THIS_ITEM() and proto_item_append_string()

["Olivier Biot (Ethereal)" <ethereal@xxxxxxxxxxxxxxx>]

Hi folks,

Since the introduction of the TRY_TO_FAKE_THIS_ITEM() speed optimizing trick 
in epan/proto.c, there were issues in dissectors calling 
proto_item_append_string() for progressive update of FT_STRING and 
FT_STRINGZ protocol fields.

As I understand it, the TRY_TO_FAKE_THIS_ITEM() speed optimization has a 
side effect in that it will only process FT_PROTOCOL fields. As a result, 
proto_item_append_string() calls may throw a dissector exception if a field 
to be appended is faked (hence returning the protocol we're in), as only a 
FT_STRING or FT_STRINGZ can be appended to with this call.

I currently checked in a fix which silently discards situations where a 
FT_PROTOCOL's value is being appended to.

Everything works as expected with this fix, but I wanted to make sure I was 
not creating trouble with the TRY_TO_FAKE_THIS_ITEM() speed optimizing 
trick.

However... I think now the following code is no longer needed in 
epan/dissectors/packet-wsp.c:
/* this function will call proto_item_append_string() which
   doesnt work with the TRY_TO_FAKE_THIS_ITEM
   speed optimization.
   So we have to disable that one and become "slow" by pretending that
   the tree is "visible".
* /
if (tree)
 PTREE_DATA(tree)->visible=1; */

I compiled Ethereal once with and once without this code snippet starting at 
line 2111, and I compared the results: they were identical.

Is it OK to remove that snippet from packet-wsp.c, and remove the 
corresponding warning (lines 1549--1560) from proto.c?

Cheers!

Olivier