Merge remote-tracking branch 'origin/development' into mueller/refactor-tmtc-stack

CHANGELOG.md

@@ -8,7 +8,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 # [unreleased]
 
-# [v5.0.0]
+# [v5.0.0] 25.07.2022
 
 ## Changes
 
@@ -23,6 +23,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
   PR: https://egit.irs.uni-stuttgart.de/fsfw/fsfw/pulls/572
 - HAL Devicehandlers: Periodic printout is run-time configurable now
 - `oneShotAction` flag in the `TestTask` class is not static anymore
+- `SimpleRingBuffer::writeData` now checks if the amount is larger than the total size of the 
+  Buffer and rejects such writeData calls with `HasReturnvaluesIF::RETURN_FAILED`
+  PR: https://egit.irs.uni-stuttgart.de/fsfw/fsfw/pulls/586
 - Major update for version handling, using `git describe` to fetch version information with git.
   PR: https://egit.irs.uni-stuttgart.de/fsfw/fsfw/pulls/601
   - Add helper functions provided by [`cmake-modules`](https://github.com/bilke/cmake-modules)

CMakeLists.txt

@@ -13,7 +13,7 @@ list(APPEND CMAKE_MODULE_PATH
 # Version file handling  #
 # ##############################################################################
 
-set(FSFW_VERSION_IF_GIT_FAILS 4)
+set(FSFW_VERSION_IF_GIT_FAILS 5)
 set(FSFW_SUBVERSION_IF_GIT_FAILS 0)
 set(FSFW_REVISION_IF_GIT_FAILS 0)
 

src/fsfw/container/SimpleRingBuffer.cpp

@@ -2,6 +2,9 @@
 
 #include <cstring>
 
+#include "fsfw/FSFW.h"
+#include "fsfw/serviceinterface.h"
+
 SimpleRingBuffer::SimpleRingBuffer(const size_t size, bool overwriteOld, size_t maxExcessBytes)
     : RingBufferBase<>(0, size, overwriteOld), maxExcessBytes(maxExcessBytes) {
   if (maxExcessBytes > size) {
@@ -48,6 +51,19 @@ void SimpleRingBuffer::confirmBytesWritten(size_t amount) {
 }
 
 ReturnValue_t SimpleRingBuffer::writeData(const uint8_t* data, size_t amount) {
+  if (data == nullptr) {
+    return HasReturnvaluesIF::RETURN_FAILED;
+  }
+  if (amount > getMaxSize()) {
+#if FSFW_VERBOSE_LEVEL >= 1
+#if FSFW_CPP_OSTREAM_ENABLED == 1
+    sif::error << "SimpleRingBuffer::writeData: Amount of data too large" << std::endl;
+#else
+    sif::printError("SimpleRingBuffer::writeData: Amount of data too large\n");
+#endif
+#endif
+    return HasReturnvaluesIF::RETURN_FAILED;
+  }
   if (availableWriteSpace() >= amount or overwriteOld) {
     size_t amountTillWrap = writeTillWrap();
     if (amountTillWrap >= amount) {

unittests/container/RingBufferTest.cpp

@@ -31,6 +31,8 @@ TEST_CASE("Ring Buffer Test", "[RingBufferTest]") {
     for (uint8_t i = 0; i < 9; i++) {
       CHECK(readBuffer[i] == i);
     }
+    REQUIRE(ringBuffer.writeData(testData, 1024) == retval::CATCH_FAILED);
+    REQUIRE(ringBuffer.writeData(nullptr, 5) == retval::CATCH_FAILED);
   }
 
   SECTION("Get Free Element Test") {
@@ -144,12 +146,13 @@ TEST_CASE("Ring Buffer Test2", "[RingBufferTest2]") {
 
   SECTION("Overflow") {
     REQUIRE(ringBuffer.availableWriteSpace() == 9);
-    // Writing more than the buffer is large, technically thats allowed
+    // We don't allow writing of Data that is larger than the ring buffer in total
-    // But it is senseless and has undesired impact on read call
+    REQUIRE(ringBuffer.getMaxSize() == 9);
-    REQUIRE(ringBuffer.writeData(testData, 13) == retval::CATCH_OK);
+    REQUIRE(ringBuffer.writeData(testData, 13) == retval::CATCH_FAILED);
-    REQUIRE(ringBuffer.getAvailableReadData() == 3);
+    REQUIRE(ringBuffer.getAvailableReadData() == 0);
     ringBuffer.clear();
     uint8_t *ptr = nullptr;
+    // With excess Bytes 13 Bytes can be written to this Buffer
     REQUIRE(ringBuffer.getFreeElement(&ptr, 13) == retval::CATCH_OK);
     REQUIRE(ptr != nullptr);
     memcpy(ptr, testData, 13);
@@ -234,11 +237,13 @@ TEST_CASE("Ring Buffer Test3", "[RingBufferTest3]") {
 
   SECTION("Overflow") {
     REQUIRE(ringBuffer.availableWriteSpace() == 9);
-    // Writing more than the buffer is large, technically thats allowed
+    // Writing more than the buffer is large.
-    // But it is senseless and has undesired impact on read call
+    // This write will be rejected and is seen as a configuration mistake
-    REQUIRE(ringBuffer.writeData(testData, 13) == retval::CATCH_OK);
+    REQUIRE(ringBuffer.writeData(testData, 13) == retval::CATCH_FAILED);
-    REQUIRE(ringBuffer.getAvailableReadData() == 3);
+    REQUIRE(ringBuffer.getAvailableReadData() == 0);
     ringBuffer.clear();
+    // Using FreeElement allows the usage of excessBytes but
+    // should be used with caution
     uint8_t *ptr = nullptr;
     REQUIRE(ringBuffer.getFreeElement(&ptr, 13) == retval::CATCH_OK);
     REQUIRE(ptr != nullptr);