Wireshark-dev: Re: [Wireshark-dev] [Wireshark-commits] master fe195c0: Don't throw for offset a
Date Index · Thread Index · Other Months · All Mailing Lists
Date Prev · Date Next · Thread Prev · Thread Next
From: Jeff Morriss <jeff.morriss.ws@xxxxxxxxx>
Date: Thu, 24 Apr 2014 17:47:25 -0400
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@xxxxxxxxx)
Changed: branch: master
Repository: wireshark

Commits:

fe195c0 by Evan Huus (eapache@xxxxxxxxx):

     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?