Wireshark-dev: Re: [Wireshark-dev] Rename TVB captured length vs reported length
From: Guy Harris <guy@xxxxxxxxxxxx>
Date: Mon, 17 Feb 2014 14:31:57 -0800
On Feb 17, 2014, at 2:07 PM, Evan Huus <eapache@xxxxxxxxx> wrote:

> After yet another patch submission where this was unclear, I would
> like to propose the following change:
> 
> tvb_length, tvb_length_remaining, etc. are changed to all operate on
> the reported length on the wire
> 
> tvb_reported_* are dropped in favor of tvb_captured_* which operate on
> the available captured length (what is currently given by just
> tvb_length).

I've been thinking the same thing over the past few years.

I think the use of un-decorated "length" stems from the original tvbuff implementation, where the length was the amount of data in the tvbuff; those tvbuffs were basically byte arrays with bounds checking, plus slicing (subset tvbuffs) and concatenation (composite tvbuffs).

I suggested adding an "actual length", so we could distinguish between "we ran past the snapshot length" and "we ran past the end of the packet" (I forget whether Gilbert or I implemented that), and the new length was called the "reported length".  This added some additional semantics, matching the behavior of packets from network captures, which, in many cases, support

That change turned tvbuffs into an abstraction that matched the semantics of packets in network captures, where a snapshot length/slice length could be specified.  They could still be used as byte arrays without those extra semantics (set captured length and reported length to the same value).

Given that

> 95% of the time the intended behaviour is best
> achieved by the reported length, but 95% of the time people new to the
> API pick up on tvb_length and friends and assume that's what they
> want.

I agree that, in retrospect, calling the "amount of data in the tvbuff" length should, in fact, have been called the captured length, with the other length being called the "reported length" or just "the length".

We might as well just call it "the length".

> I realize this is a subtly breaking behavioral change to the
> API,

Yes - the change probably can't be done completely automatically.

Perhaps step 1 is to rename tvb_length() to tvb_captured_length() without any alias and fix all references to tvb_length() either to be tvb_captured_length() or tvb_reported_length() and, once that's done, rename tvb_reported_length() to tvb_length().

> but I figure in the long run it will make a lot of things much
> simpler.

Yes.

> It also gives a mental model which is IMHO slightly nicer:
> the TVB represents the entire packet with potentially-incomplete
> backing data, instead of representing the backing data of a
> potentially-bigger-on-the-inside packet.

Yes, that's essentially what tvbuffs have been since we added the reported length; we might as well acknowledge it.