Jeff (and/or anyone else who cares) I'd appreciate a verification that my logic here is correct, and that I've not accidentally undone your good work.
Thanks, Evan On Sun, Oct 13, 2013 at 12:54 AM, <[email protected]> wrote: > http://anonsvn.wireshark.org/viewvc/viewvc.cgi?view=rev&revision=52578 > > User: eapache > Date: 2013/10/13 04:54 AM > > Log: > So a while back Jeff added some code to check that the offset+length passed > to > proto_tree_add_item was valid *before* we short-circuited based on a NULL > tree. > This was good in that it removed a common source of really-long-loop bugs. It > was less good in that it cost us about 8% in speed when doing a tree-less > dissection, but we decided the tradeoff was worth it. > > After Anders' recent mail to -dev about performance, I started thinking about > how to optimize this. It occurred to me that the vast majority of the logic > involved in the check was dealing with the length value - fetching the actual > length if it was a counted string, calculating the length if it was -1, > adding > the length to the offset in a way that was free from overflows, etc. > > All of this is (theoretically) unnecessary - simply checking the offset > without > worrying about the length will still catch the very-long-loops, since it is > the > offset that increases in each iteration, not the length. > > All that to justify: > - implement tvb_ensure_offset_exists which throws an exception if the offset > is > not within the tvb > - use it instead of all the complicated other logic in the pre-short-circuit > step of proto_tree_add_item and friends > > This gives us back about 3/4 of the performance we lost in the original > patch. > We're still ~2% slower than without any check, but this is the best I can > think > of right now. > > Directory: /trunk/epan/ > Changes Path Action > +8 -44 proto.c Modified > +17 -0 tvbuff.c Modified > +3 -0 tvbuff.h Modified > > ___________________________________________________________________________ > Sent via: Wireshark-commits mailing list <[email protected]> > Archives: http://www.wireshark.org/lists/wireshark-commits > Unsubscribe: https://wireshark.org/mailman/options/wireshark-commits > > mailto:[email protected]?subject=unsubscribe ___________________________________________________________________________ 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
