Wireshark-commits: [Wireshark-commits] rev 52578: /trunk/epan/ /trunk/epan/: proto.c tvbuff.c tvbuf
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