buffer: bugfixes and cleanups
- Added clarifying comments regarding the different offsets
- Fixed an instance in wraparoundRead where we updated the readPtr
with the relative offset
- Added additional checks in initialization to check for buffer size
- Use the queueSize for buffer initialization opposed to the memory
region size for the whole MMIO
Tested: Unit tested
Signed-off-by: Brandon Kim <brandonkim@google.com>
Change-Id: Ib175ed1c9c129c0f9d2b4f9737e2d394f160434d
diff --git a/test/buffer_test.cpp b/test/buffer_test.cpp
index 7218f8d..8649c27 100644
--- a/test/buffer_test.cpp
+++ b/test/buffer_test.cpp
@@ -62,19 +62,39 @@
TEST_F(BufferTest, BufferInitializeEraseFail)
{
InSequence s;
+ EXPECT_CALL(*dataInterfaceMockPtr, getMemoryRegionSize())
+ .WillOnce(Return(testRegionSize));
+ EXPECT_THROW(
+ try {
+ // Test too big of a proposed buffer compared to the memori size
+ uint16_t bigQueueSize = 0x181;
+ uint16_t bigUeRegionSize = 0x50;
+ bufferImpl->initialize(testBmcInterfaceVersion, bigQueueSize,
+ bigUeRegionSize, testMagicNumber);
+ } catch (const std::runtime_error& e) {
+ EXPECT_STREQ(
+ e.what(),
+ "[initialize] Proposed region size '513' "
+ "is bigger than the BMC's allocated MMIO region of '512'");
+ throw;
+ },
+ std::runtime_error);
+ EXPECT_NE(bufferImpl->getCachedBufferHeader(), testInitializationHeader);
EXPECT_CALL(*dataInterfaceMockPtr, getMemoryRegionSize())
.WillOnce(Return(testRegionSize));
- const std::vector<uint8_t> emptyArray(testRegionSize, 0);
+ size_t testProposedBufferSize =
+ sizeof(struct CircularBufferHeader) + testUeRegionSize + testQueueSize;
+ const std::vector<uint8_t> emptyArray(testProposedBufferSize, 0);
// Return a smaller write than the intended testRegionSize to test the error
EXPECT_CALL(*dataInterfaceMockPtr, write(0, ElementsAreArray(emptyArray)))
- .WillOnce(Return(testRegionSize - 1));
+ .WillOnce(Return(testProposedBufferSize - 1));
EXPECT_THROW(
try {
bufferImpl->initialize(testBmcInterfaceVersion, testQueueSize,
testUeRegionSize, testMagicNumber);
} catch (const std::runtime_error& e) {
- EXPECT_STREQ(e.what(), "Buffer initialization only erased '511'");
+ EXPECT_STREQ(e.what(), "[initialize] Only erased '383'");
throw;
},
std::runtime_error);
@@ -83,7 +103,7 @@
EXPECT_CALL(*dataInterfaceMockPtr, getMemoryRegionSize())
.WillOnce(Return(testRegionSize));
EXPECT_CALL(*dataInterfaceMockPtr, write(0, ElementsAreArray(emptyArray)))
- .WillOnce(Return(testRegionSize));
+ .WillOnce(Return(testProposedBufferSize));
// Return a smaller write than the intended initializationHeader to test the
// error
EXPECT_CALL(*dataInterfaceMockPtr, write(0, _)).WillOnce(Return(0));
@@ -92,9 +112,8 @@
bufferImpl->initialize(testBmcInterfaceVersion, testQueueSize,
testUeRegionSize, testMagicNumber);
} catch (const std::runtime_error& e) {
- EXPECT_STREQ(
- e.what(),
- "Buffer initialization buffer header write only wrote '0'");
+ EXPECT_STREQ(e.what(),
+ "[initialize] Only wrote '0' bytes of the header");
throw;
},
std::runtime_error);
@@ -106,9 +125,11 @@
InSequence s;
EXPECT_CALL(*dataInterfaceMockPtr, getMemoryRegionSize())
.WillOnce(Return(testRegionSize));
- const std::vector<uint8_t> emptyArray(testRegionSize, 0);
+ size_t testProposedBufferSize =
+ sizeof(struct CircularBufferHeader) + testUeRegionSize + testQueueSize;
+ const std::vector<uint8_t> emptyArray(testProposedBufferSize, 0);
EXPECT_CALL(*dataInterfaceMockPtr, write(0, ElementsAreArray(emptyArray)))
- .WillOnce(Return(testRegionSize));
+ .WillOnce(Return(testProposedBufferSize));
uint8_t* testInitializationHeaderPtr =
reinterpret_cast<uint8_t*>(&testInitializationHeader);
@@ -193,10 +214,12 @@
InSequence s;
EXPECT_CALL(*dataInterfaceMockPtr, getMemoryRegionSize())
.WillOnce(Return(testRegionSize));
- const std::vector<uint8_t> emptyArray(testRegionSize, 0);
+ size_t testProposedBufferSize = sizeof(struct CircularBufferHeader) +
+ testUeRegionSize + testQueueSize;
+ const std::vector<uint8_t> emptyArray(testProposedBufferSize, 0);
EXPECT_CALL(*dataInterfaceMockPtr,
write(0, ElementsAreArray(emptyArray)))
- .WillOnce(Return(testRegionSize));
+ .WillOnce(Return(testProposedBufferSize));
uint8_t* testInitializationHeaderPtr =
reinterpret_cast<uint8_t*>(&testInitializationHeader);
@@ -216,17 +239,15 @@
TEST_F(BufferWraparoundReadTest, TooBigReadFail)
{
InSequence s;
- EXPECT_CALL(*dataInterfaceMockPtr, getMemoryRegionSize())
- .WillOnce(Return(testRegionSize));
- size_t tooBigLength = testRegionSize - expectedqueueOffset + 1;
+ size_t tooBigLength = testQueueSize + 1;
EXPECT_THROW(
try {
bufferImpl->wraparoundRead(/* offset */ 0, tooBigLength);
} catch (const std::runtime_error& e) {
- EXPECT_STREQ(e.what(),
- "[wraparoundRead] queueOffset '128' + length '385' + "
- "additionalBoundaryCheck '0' + was "
- "bigger than memoryRegionSize '512'");
+ EXPECT_STREQ(
+ e.what(),
+ "[wraparoundRead] length '257' + additionalBoundaryCheck '0' "
+ "was bigger than queueSize '256'");
throw;
},
std::runtime_error);
@@ -235,21 +256,18 @@
TEST_F(BufferWraparoundReadTest, TooBigReadWithAdditionalBoundaryCheckFail)
{
InSequence s;
- EXPECT_CALL(*dataInterfaceMockPtr, getMemoryRegionSize())
- .WillOnce(Return(testRegionSize));
- // Use additionalBoundaryCheck to still go over the memoryRegionSize by 1
+ // Use additionalBoundaryCheck to still go over the queueSize by 1
size_t additionalBoundaryCheck = 10;
- size_t tooBigLength =
- testRegionSize - expectedqueueOffset - additionalBoundaryCheck + 1;
+ size_t tooBigLength = testQueueSize - additionalBoundaryCheck + 1;
EXPECT_THROW(
try {
bufferImpl->wraparoundRead(/* offset */ 0, tooBigLength,
additionalBoundaryCheck);
} catch (const std::runtime_error& e) {
- EXPECT_STREQ(e.what(),
- "[wraparoundRead] queueOffset '128' + length '375' + "
- "additionalBoundaryCheck '10' + was "
- "bigger than memoryRegionSize '512'");
+ EXPECT_STREQ(
+ e.what(),
+ "[wraparoundRead] length '247' + additionalBoundaryCheck '10' "
+ "was bigger than queueSize '256'");
throw;
},
std::runtime_error);
@@ -258,8 +276,6 @@
TEST_F(BufferWraparoundReadTest, NoWrapAroundReadPass)
{
InSequence s;
- EXPECT_CALL(*dataInterfaceMockPtr, getMemoryRegionSize())
- .WillOnce(Return(testRegionSize));
size_t testLength = 0x10;
size_t testOffset = 0x50;
@@ -282,11 +298,9 @@
TEST_F(BufferWraparoundReadTest, WrapAroundReadFails)
{
InSequence s;
- EXPECT_CALL(*dataInterfaceMockPtr, getMemoryRegionSize())
- .WillOnce(Return(testRegionSize));
size_t testBytesLeft = 3;
size_t testLength = 0x10;
- size_t testOffset = testRegionSize - (testLength - testBytesLeft);
+ size_t testOffset = testQueueSize - (testLength - testBytesLeft);
// Read 3 bytes short
std::vector<std::uint8_t> testBytesReadShort(testLength - testBytesLeft);
@@ -314,11 +328,9 @@
TEST_F(BufferWraparoundReadTest, WrapAroundReadPasses)
{
InSequence s;
- EXPECT_CALL(*dataInterfaceMockPtr, getMemoryRegionSize())
- .WillOnce(Return(testRegionSize));
size_t testBytesLeft = 3;
size_t testLength = 0x10;
- size_t testOffset = testRegionSize - (testLength - testBytesLeft);
+ size_t testOffset = testQueueSize - (testLength - testBytesLeft);
// Read 3 bytes short
std::vector<std::uint8_t> testBytesReadFirst{16, 15, 14, 13, 12, 11, 10,
@@ -332,7 +344,7 @@
// Call to updateReadPtr is triggered
const std::vector<uint8_t> expectedReadPtr{
- static_cast<uint8_t>(expectedqueueOffset + testBytesLeft), 0x0};
+ static_cast<uint8_t>(testBytesLeft), 0x0};
EXPECT_CALL(*dataInterfaceMockPtr, write(expectedBmcReadPtrOffset,
ElementsAreArray(expectedReadPtr)))
.WillOnce(Return(expectedWriteSize));
@@ -357,8 +369,6 @@
void wraparoundReadMock(std::span<std::uint8_t> expetedBytesOutput)
{
- EXPECT_CALL(*dataInterfaceMockPtr, getMemoryRegionSize())
- .WillOnce(Return(testRegionSize));
EXPECT_CALL(*dataInterfaceMockPtr, read(_, _))
.WillOnce(Return(std::vector<std::uint8_t>(
expetedBytesOutput.begin(), expetedBytesOutput.end())));