I just submitted a patch for review that completely removes the private_data member of the packet_info structure:
I'm mentioning it here for better visibility in case there are objections. I've reworked all of the dissector code in the Wireshark master branch (mostly to pass the data through the dissector data parameter of "new style" dissectors), but obviously don't have any insight into use by proprietary plugins (although I would recommend it be removed from there too)
One of the reasons I started working on removing the private_data member (besides the potential bugs it could create), was that I saw many dissectors using a TRY/CATCH block to restore the value of the private_data member. I'm not really sure how many were actually 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 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?
Michael
