pombreda / libjingle

Automatically exported from code.google.com/p/libjingle
0 stars 0 forks source link

FifoBuffer::ConsumeWriteBuffer incorrectly calculates buffer size on full buffer. #254

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
This is a bug that is probably not exercised in libjingle code, but it does 
cause problems if you try to use libjingle's FifoBuffer.

What steps will reproduce the problem?

If the FifoBuffer is full, GetWriteBuffer reports the frame as empty. The 
following gtest reproduces the problem:

TEST(FifoBufferTest, FullBufferCheck)
{ 
  // The upstream FifoBuffer implementation has a logic error that causes a
  // full buffer to sometimes be reported as an empty buffer.

  FifoBuffer buff(10);
  buff.ConsumeWriteBuffer(10);

  size_t free;                            
  buff.GetWriteBuffer(&free);             
  EXPECT_EQ(0U, free);
}   

What is the expected output? What do you see instead?

I expect the test to pass, but it fails:
[ RUN      ] FifoBufferTest.FullBufferCheck
/st/stor1/blaise/texas-relay/stacks/texas_videoconf/vcclient/tests/relay/test_fi
fo_buffer.cpp:312: Failure
Value of: free
  Actual: 10
Expected: 0U
Which is: 0
[  FAILED  ] FifoBufferTest.FullBufferCheck (0 ms)
[----------] 3 tests from FifoBufferTest (0 ms total)

What version of the product are you using? On what operating system?
Not OS dependent. The libjingle that exhibits this is a few months old. I don't 
have the exact version, but can track it down if necessary.

Please provide any additional information below.

The following formula for *size in FifoBuffer::ConsumeWriteBuffer should fix 
the problem:
  *size = std::min(buffer_length_ - write_position, buffer_length_ - data_length_);

Original issue reported on code.google.com by bla...@suitabletech.com on 8 Dec 2011 at 8:49

GoogleCodeExporter commented 9 years ago
Jun, can you fix this issue? Should be a 1-line fix.

Original comment by juberti@google.com on 8 Dec 2011 at 9:29

GoogleCodeExporter commented 9 years ago

Original comment by jun...@google.com on 9 Dec 2011 at 12:55

GoogleCodeExporter commented 9 years ago
It's fixed. Should be in the next release 0.6.7

Original comment by jun...@google.com on 22 Dec 2011 at 10:03