Re: Removal of pinfo->private_data

[Jeff Morriss <jeff.morriss.ws () gmail com>]

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