Merge pull request #1 from kbotteon/botteon/fix-buffermanager

Fix BufferManager bounds checking logic
timcanham · Jul 20, 2021 · b5ff59e · b5ff59e
2 parents 151b6fc + cd9c2c5
commit b5ff59e
Show file tree

Hide file tree

Showing 2 changed files with 14 additions and 11 deletions.
diff --git a/Svc/BufferManager/BufferManagerComponentImpl.cpp b/Svc/BufferManager/BufferManagerComponentImpl.cpp
@@ -123,7 +123,7 @@ namespace Svc {
       FW_ASSERT(m_buffers);
       // find smallest buffer based on size.
       for (NATIVE_UINT_TYPE buff = 0; buff < this->m_numStructs; buff++) {
-          if ((not this->m_buffers[buff].allocated) and (size < this->m_buffers[buff].size)) {
+          if ((not this->m_buffers[buff].allocated) and (size <= this->m_buffers[buff].size)) {
               this->m_buffers[buff].allocated = true;
               this->m_currBuffs++;
               if (this->m_currBuffs > this->m_highWater) {
@@ -203,8 +203,11 @@ namespace Svc {
         }
     }
 
-    // check some assertions
-    FW_ASSERT(bufferMem == (static_cast<U8*>(memory) + memorySize));
+    // check that the initiation pointer made it to the end of allocated space
+    U8* const CURR_PTR = bufferMem;
+    U8* const END_PTR = static_cast<U8*>(memory) + memorySize;
+    FW_ASSERT(CURR_PTR == END_PTR, reinterpret_cast<U64>(CURR_PTR), reinterpret_cast<U64>(END_PTR));
+    // secondary init verification
     FW_ASSERT(currStruct == this->m_numStructs,currStruct,this->m_numStructs);
     // indicate setup is done
     this->m_setup = true;

diff --git a/Svc/BufferManager/test/ut/Tester.cpp b/Svc/BufferManager/test/ut/Tester.cpp
@@ -206,7 +206,7 @@ namespace Svc {
 
       for (NATIVE_UINT_TYPE b=0; b<BIN1_NUM_BUFFERS; b++) {
           // Get the buffers
-          buffs[b] = this->invoke_to_bufferGetCallee(0,BIN1_BUFFER_SIZE-1);
+          buffs[b] = this->invoke_to_bufferGetCallee(0,BIN1_BUFFER_SIZE);
           // check allocation state
           ASSERT_TRUE(this->component.m_buffers[b].allocated);
           // check stats
@@ -217,7 +217,7 @@ namespace Svc {
 
 
       // should send back empty buffer
-      Fw::Buffer noBuff = this->invoke_to_bufferGetCallee(0,BIN1_BUFFER_SIZE-1);
+      Fw::Buffer noBuff = this->invoke_to_bufferGetCallee(0,BIN1_BUFFER_SIZE);
       ASSERT_EQ(1,this->component.m_noBuffs);
 
       // check telemetry
@@ -311,7 +311,7 @@ namespace Svc {
 
       for (NATIVE_UINT_TYPE b=0; b<BIN0_NUM_BUFFERS+BIN1_NUM_BUFFERS+BIN2_NUM_BUFFERS; b++) {
           // Get the buffers
-          buffs[b] = this->invoke_to_bufferGetCallee(0,BIN0_BUFFER_SIZE-1);
+          buffs[b] = this->invoke_to_bufferGetCallee(0,BIN0_BUFFER_SIZE);
           // check allocation state
           ASSERT_TRUE(this->component.m_buffers[b].allocated);
           // check stats
@@ -323,7 +323,7 @@ namespace Svc {
       REQUIREMENT("FPRIME-BM-003");
 
       // should send back empty buffer
-      Fw::Buffer noBuff = this->invoke_to_bufferGetCallee(0,BIN1_BUFFER_SIZE-1);
+      Fw::Buffer noBuff = this->invoke_to_bufferGetCallee(0,BIN1_BUFFER_SIZE);
       ASSERT_EQ(1,this->component.m_noBuffs);
 
       // check telemetry
@@ -385,15 +385,15 @@ namespace Svc {
 
       for (NATIVE_UINT_TYPE b=0; b<BIN1_NUM_BUFFERS+BIN2_NUM_BUFFERS; b++) {
           // Get the buffers
-          buffs[b] = this->invoke_to_bufferGetCallee(0,BIN1_BUFFER_SIZE-1);
+          buffs[b] = this->invoke_to_bufferGetCallee(0,BIN1_BUFFER_SIZE);
           // check allocation state - should be allocating from bin 1
           ASSERT_TRUE(this->component.m_buffers[b+BIN0_NUM_BUFFERS].allocated);
           // check stats
           ASSERT_EQ(b+1,this->component.m_currBuffs);
       }
 
       // should send back empty buffer
-      noBuff = this->invoke_to_bufferGetCallee(0,BIN1_BUFFER_SIZE-1);
+      noBuff = this->invoke_to_bufferGetCallee(0,BIN1_BUFFER_SIZE);
       ASSERT_EQ(2,this->component.m_noBuffs);
 
       // check telemetry
@@ -444,15 +444,15 @@ namespace Svc {
 
       for (NATIVE_UINT_TYPE b=0; b<BIN2_NUM_BUFFERS; b++) {
           // Get the buffers
-          buffs[b] = this->invoke_to_bufferGetCallee(0,BIN2_BUFFER_SIZE-1);
+          buffs[b] = this->invoke_to_bufferGetCallee(0,BIN2_BUFFER_SIZE);
           // check allocation state - should be allocating from bin 1
           ASSERT_TRUE(this->component.m_buffers[b+BIN0_NUM_BUFFERS+BIN1_NUM_BUFFERS].allocated);
           // check stats
           ASSERT_EQ(b+1,this->component.m_currBuffs);
       }
 
       // should send back empty buffer
-      noBuff = this->invoke_to_bufferGetCallee(0,BIN2_BUFFER_SIZE-1);
+      noBuff = this->invoke_to_bufferGetCallee(0,BIN2_BUFFER_SIZE);
       ASSERT_EQ(3,this->component.m_noBuffs);
 
       // check telemetry