Opened 12 years ago

Closed 12 years ago

Last modified 12 years ago

#870 closed enhancement (fixed)

Include assertion in TComOutputBitstream::write()

Reported by: adarsh Owned by:
Priority: minor Milestone: HM-9.2
Component: HM Version: HM-9.0
Keywords: Cc: fbossen, ksuehring, davidf, jct-vc@…

Description

There is no check in TComOutputBitstream::write(UInt uiBits, UInt uiNumberOfBits) function to see if the size of the variable to be written, uiBits, does not exceed uiNumberOfBits bits. I suggest an assertion be included so that even if someone accidentally assigns a larger value to uiBits, this assertion would catch it.

If the size of uiBits is more than uiNumberOfBits bits, (the way the write() function is currently written) there may be no errors thrown by the code, and some of the preceding syntax elements may have wrong values when decoding the bitstream. The attached patch adds the assertion to the write() function.

Attachments (1)

writefunction_bitoverflow_870.patch​ (837 bytes) - added by adarsh 12 years ago.

Download all attachments as: .zip

Change History (7)

comment:1 Changed 12 years ago by DefaultCC Plugin

  • Cc fbossen ksuehring davidf jct-vc@… added

Changed 12 years ago by adarsh

comment:2 Changed 12 years ago by davidf

Surely assert(0 == (uiBits & MAX_UINT << uiNumberOfBits)) is a simpler expression?

comment:3 Changed 12 years ago by adarsh

That is indeed much simpler. No subtractions and "<=" needed.

comment:4 Changed 12 years ago by fbossen

  • Milestone set to HM-9.2
  • Resolution set to fixed
  • Status changed from new to closed
  • Type changed from defect to enhancement

Fixed in r3191

comment:5 Changed 12 years ago by adarsh

There is a problem with the fix when uiNumberOfBits = 32, the size of Int variable.

assert(0 == (uiBits & MAX_UINT << uiNumberOfBits))

It (looks like before left shift, a mod operation is done on "uiNumberOfBits", which is uiNumberOfBits % 32) which becomes 0 when uiNumberOfBits = 32. And this would result in MAX_UINT not being shifted at all (the intention is to shift it left by 32 bits, resulting in zero).

comment:6 Changed 12 years ago by fbossen

uiNumberOfBits = 32 addressed in r3198

Note: See TracTickets for help on using tickets.

This list contains all users that will be notified about changes made to this ticket.

These roles will be notified: Reporter, Owner, Subscriber, Participant

  • Adarsh Krishnan Ramasubramonian(Reporter, Participant)
  • David Flynn(Subscriber, Participant)
  • Frank Bossen(Subscriber, Participant)
  • jct-vc@…(Subscriber)
  • karl.sharman@…(Always)
  • Karsten Suehring(Subscriber, Always)