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, <eapache@xxxxxxxxxxxxx> 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 <wireshark-commits@xxxxxxxxxxxxx>
> Archives: http://www.wireshark.org/lists/wireshark-commits
> Unsubscribe: https://wireshark.org/mailman/options/wireshark-commits
> mailto:wireshark-commits-request@xxxxxxxxxxxxx?subject=unsubscribe
