I just submitted a patch for review that completely removes the private_data 
member of the packet_info structure:

https://code.wireshark.org/review/5487

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

___________________________________________________________________________
Sent via:    Wireshark-dev mailing list <[email protected]>
Archives:    http://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
             mailto:[email protected]?subject=unsubscribe

Reply via email to