commit 613ba537eba3b61c8ecbde2fd82d66a3591fe035
author Brandon Kim <brandonkim@google.com> Mon Sep 12 20:55:21 2022 +0000
committer Brandon Kim <brandonkim@google.com> Mon Sep 12 23:07:06 2022 +0000
tree 917a4968dd36dae8e4a8b4012ff4d85576bc9e0e
parent 9a895acd6bffc7d42c69677ea6f8f4bdf8ea24da

buffer: Fix a corner case with header wraparound

Update the unit test so that we cover this corner case + update the API
so that we no longer pass in the offset manually.

Tested: Unit test coverage + integration test with our downstream
platform that found this issue

Signed-off-by: Brandon Kim <brandonkim@google.com>
Change-Id: I6b0a7974f902e0248bfb4932237176df20fdaf61

test/buffer_test.cpp

diff --git a/test/buffer_test.cpp b/test/buffer_test.cpp
index fce322b..0fff5ee 100644
--- a/test/buffer_test.cpp
+++ b/test/buffer_test.cpp
@@ -503,7 +503,7 @@
     // Calculated checksum for the header
     static constexpr uint8_t testChecksum =
         (testSequenceId ^ testEntrySize ^ testRdeCommandType);
-    size_t testOffset = 0x20;
+    size_t testOffset = 0x0;
 
     struct QueueEntryHeader testEntryHeader
     {};
@@ -515,7 +515,7 @@
     std::vector<uint8_t> testEntryHeaderVector(
         testEntryHeaderPtr, testEntryHeaderPtr + entryHeaderSize);
     wraparoundReadMock(testOffset, testEntryHeaderVector);
-    EXPECT_EQ(bufferImpl->readEntryHeader(testOffset), testEntryHeader);
+    EXPECT_EQ(bufferImpl->readEntryHeader(), testEntryHeader);
     // Check the bmcReadPtr
     struct CircularBufferHeader cachedBufferHeader =
         bufferImpl->getCachedBufferHeader();
@@ -535,9 +535,7 @@
     wraparoundReadMock(testOffset, testEntryHeaderVector);
     wraparoundReadMock(testOffset + entryHeaderSize, testEntryVector);
     EXPECT_THROW(
-        try {
-            bufferImpl->readEntry(testOffset);
-        } catch (const std::runtime_error& e) {
+        try { bufferImpl->readEntry(); } catch (const std::runtime_error& e) {
             // Calculation: testChecksum (0x21) XOR (0x22) = 3
             EXPECT_STREQ(e.what(),
                          "[readEntry] Checksum was '3', expected '0'");
@@ -546,7 +544,7 @@
         std::runtime_error);
 }
 
-TEST_F(BufferEntryTest, ReadEntryPass)
+TEST_F(BufferEntryTest, ReadEntryPassWraparound)
 {
     InSequence s;
     // We expect this will bump checksum up by "testEntrySize" = 0xff ^ 0xff ...
@@ -555,20 +553,42 @@
     uint8_t* testEntryHeaderPtr = reinterpret_cast<uint8_t*>(&testEntryHeader);
     std::vector<uint8_t> testEntryHeaderVector(
         testEntryHeaderPtr, testEntryHeaderPtr + entryHeaderSize);
-    // Set testOffset so that we can test the wraparound here as well on our
-    // second read for the entry (by 1 byte)
-    testOffset = testQueueSize - entryHeaderSize - 1;
+    // Set testOffset so that we can test the wraparound here at the header and
+    // update the readPtr
+    testOffset = testQueueSize - 1;
+    EXPECT_CALL(*dataInterfaceMockPtr, write(expectedBmcReadPtrOffset, _))
+        .WillOnce(Return(expectedWriteSize));
+    EXPECT_NO_THROW(bufferImpl->updateReadPtr(testOffset));
+
     wraparoundReadMock(testOffset, testEntryHeaderVector);
     wraparoundReadMock(testOffset + entryHeaderSize, testEntryVector);
 
     EntryPair testedEntryPair;
-    EXPECT_NO_THROW(testedEntryPair = bufferImpl->readEntry(testOffset));
+    EXPECT_NO_THROW(testedEntryPair = bufferImpl->readEntry());
     EXPECT_EQ(testedEntryPair.first, testEntryHeader);
     EXPECT_THAT(testedEntryPair.second, ElementsAreArray(testEntryVector));
     struct CircularBufferHeader cachedBufferHeader =
         bufferImpl->getCachedBufferHeader();
     // The bmcReadPtr should have been updated to reflect the wraparound
     EXPECT_EQ(boost::endian::little_to_native(cachedBufferHeader.bmcReadPtr),
+              entryHeaderSize + testEntrySize - 1);
+
+    // Set testOffset so that we can test the wraparound here as well on our
+    // second read for the entry (by 1 byte)
+    testOffset = testQueueSize - entryHeaderSize - 1;
+    EXPECT_CALL(*dataInterfaceMockPtr, write(expectedBmcReadPtrOffset, _))
+        .WillOnce(Return(expectedWriteSize));
+    EXPECT_NO_THROW(bufferImpl->updateReadPtr(testOffset));
+
+    wraparoundReadMock(testOffset, testEntryHeaderVector);
+    wraparoundReadMock(testOffset + entryHeaderSize, testEntryVector);
+
+    EXPECT_NO_THROW(testedEntryPair = bufferImpl->readEntry());
+    EXPECT_EQ(testedEntryPair.first, testEntryHeader);
+    EXPECT_THAT(testedEntryPair.second, ElementsAreArray(testEntryVector));
+    cachedBufferHeader = bufferImpl->getCachedBufferHeader();
+    // The bmcReadPtr should have been updated to reflect the wraparound
+    EXPECT_EQ(boost::endian::little_to_native(cachedBufferHeader.bmcReadPtr),
               testEntrySize - 1);
 }