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.