Wireshark mailing list archives
Re: Removal of pinfo->private_data
From: Jeff Morriss <jeff.morriss.ws () gmail com>
Date: Wed, 26 Nov 2014 11:42:48 -0500
On 11/25/14 11:43, mmann78 () netscape net 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.
___________________________________________________________________________ Sent via: Wireshark-dev mailing list <wireshark-dev () wireshark org> Archives: http://www.wireshark.org/lists/wireshark-dev Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev mailto:wireshark-dev-request () wireshark org?subject=unsubscribe
Current thread:
- Removal of pinfo->private_data mmann78 (Nov 25)
- Re: Removal of pinfo->private_data Jeff Morriss (Nov 26)