Re: [Wireshark-commits] rev 52578: /trunk/epan/ /trunk/epan/: proto.c tvbuff.c tvbuff.h

[Evan Huus <eapache () gmail com>]

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