Re: [Wireshark-commits] master fe195c0: Don't throw for offset at end of TVB with len -1.

[Jeff Morriss <jeff.morriss.ws () gmail com>]

On 04/23/2014 11:57 AM, Wireshark code review wrote:
> URL: https://code.wireshark.org/review/gitweb?p=wireshark.git;a=commit;h=fe195c0c978b4b92dc5a77daa6449a7f1314243d
> Submitter: Evan Huus (eapache () gmail com)
> Changed: branch: master
> Repository: wireshark
>
> Commits:
>
> fe195c0 by Evan Huus (eapache () gmail com):
>
>      Don't throw for offset at end of TVB with len -1.
>
>      g867a1827e7dc88896ee27a107eb35c4b3973d270 introduced a change to cleanup/fix
>      handling of bounds checks for -1 length fields, but it ended up guaranteeing a
>      throw for 0-length tvbs, which isn't good; we ought to be able to add 0-length
>      FT_PROTOCOL items at the very least.

Well nuts...

>      Better names for the function than _cheat are welcome, but I want to shut up the
>      buildbot.

What I'm thinking at the moment is your new function should become 
tvb_captured_length_remaining() (so: throwing an exception returns the 
old -1 return value).  I fail to see any real benefit in not throwing an 
exception if someone's offset is beyond the end of a TVB (of course I 
also haven't dug through the uses of the function yet). 
tvb_ensure_captured_length_remaining() then remains to ensure there's at 
least one byte there (though I admit I do have some doubts about whether 
that's really necessary; if a caller wants to ensure there's one byte 
there then they could/should just call tvb_captured_length_remaining() 
with an offset of offset+1).

Thoughts?

___________________________________________________________________________
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