Huge thanks to our Platinum Members Endace and LiveAction,
and our Silver Member Veeam, for supporting the Wireshark Foundation and project.

Wireshark-dev: Re: [Wireshark-dev] Removal of pinfo->private_data

From: Jeff Morriss <jeff.morriss.ws@xxxxxxxxx>
Date: Wed, 26 Nov 2014 11:42:48 -0500
On 11/25/14 11:43, mmann78@xxxxxxxxxxxx wrote:
necessary and how many were "copy/paste imitators".  Some also
copy/pasted the same comment that was (paraphrasing) "Don't let
exceptions thrown by subdissectors get in our way of continuing to
dissect".  I've always thought of TRY/CATCH blocks as a "performance
hit" and that they should be avoided whenever possible.  However, when I

I'm glad I'm not the only one who has thought that about the performance impact. Though I think the last time I said it someone questioned whether it's really true or not (anyway I now have a question in the back of my mind about its validity).

was cleaning up the "save & restore uses" of pinfo->private_data
(https://code.wireshark.org/review/5486/), a quick glance at the code
around it gave me the impression the TRY/CATCH blocks may still be
legitimate.
There are still other dissectors that have TRY/CATCH blocks to restore
other members of the packet_info structure (so I guess those have to
stay), but I wasn't sure if now would be the time to evaluate the use of
the TRY/CATCH blocks and possibly eliminate some.  What should the
reasons be for a dissector to use a TRY/CATCH block and is just "fear of
a malformed packet in a subdissector's tvb" enough?  Should dissectors
some how be "protected" before/after calls to call_dissector() (and
similar) so they can do their "save & restore" before/after that call?

That was the reason for at least some of them, especially the saving and restoration of private_data. I started adding that years ago because I ran into a situation where a subdissector had modified private_data (changing it from a pointer to an integer) and then threw an exception (so it failed to restore private_data to its old value). Of course someone later accessed private_data assuming it to still be a pointer which resulted in a segmentation violation.

(My assumption was other dissectors might have the same problem though it might end in a less violent but also much less clear way: private_data might still be a pointer but it might point to something wrong leading to really weird behavior.)

Having an automatic push/pop of (copies of) pinfo certainly would make it much less error prone. Though it also might cost more in performance, at least for normal captures where exceptions aren't common.