From a330a0c9072f029bc4d1b2e8832052c108695e98 Mon Sep 17 00:00:00 2001
From: Dimitrios Stoupis <dimitris.apple@gmail.com>
Date: Thu, 22 Nov 2018 11:46:57 +0000
Subject: [PATCH] Implement the octect string manipulations in a more memory
safe way
---
inc/Message.hpp | 15 ++++++---------
src/Services/MemoryManagementService.cpp | 22 ++++------------------
test/Services/MemoryManagementService.cpp | 8 ++++----
3 files changed, 14 insertions(+), 31 deletions(-)
diff --git a/inc/Message.hpp b/inc/Message.hpp
index d471cd36..a6ad919d 100644
--- a/inc/Message.hpp
+++ b/inc/Message.hpp
@@ -400,20 +400,17 @@ public:
/**
* Fetches a N-byte string from the current position in the message
*
+ * @details In the current implementation we assume that a preallocated array of
+ * sufficient size is provided as the argument
+ * @todo Specify if the provided array size is too small or too large
+ *
* PTC = 7, PFC = 0
*/
- uint8_t *readOctetString(uint16_t *length) {
+ uint16_t readOctetString(uint8_t *byteString) {
uint16_t size = readUint16(); // Get the data length from the message
- uint8_t *byteString = nullptr; // Pointer to store the string
-
- byteString = static_cast<uint8_t *>(malloc(size));
- if (byteString == nullptr) {
- return nullptr;
- }
readString(byteString, size); // Read the string data
- *length = size; // Save the data length to the provided variable
- return byteString; // Return the read string pointer
+ return size; // Return the read string pointer
}
/**
diff --git a/src/Services/MemoryManagementService.cpp b/src/Services/MemoryManagementService.cpp
index 26de5566..cd84b3dd 100644
--- a/src/Services/MemoryManagementService.cpp
+++ b/src/Services/MemoryManagementService.cpp
@@ -26,7 +26,7 @@ void MemoryManagementService::RawDataMemoryManagement::loadRawData(Message &requ
assert(request.messageType == 2);
// Variable declaration
- uint16_t dataLength = 0; // Data length to load
+ uint8_t readData[ECSS_MAX_STRING_SIZE]; // Preallocate the array
uint8_t memoryID = request.readEnum8(); // Read the memory ID from the request
uint16_t iterationCount = request.readUint16(); // Get the iteration count
@@ -34,7 +34,7 @@ void MemoryManagementService::RawDataMemoryManagement::loadRawData(Message &requ
if (memoryID == MemoryManagementService::MemoryID::RAM) {
for (std::size_t j = 0; j < iterationCount; j++) {
uint64_t startAddress = request.readUint64(); // Start address of the memory
- uint8_t *readData = request.readOctetString(&dataLength);
+ uint16_t dataLength = request.readOctetString(readData); // Data length to load
// todo: Error logging has to be included, if memory allocation above fails
// todo: Continue only if the checksum passes (when the checksum will be implemented)
@@ -56,7 +56,7 @@ void MemoryManagementService::RawDataMemoryManagement::dumpRawData(Message &requ
Message report = mainService.createTM(6);
// Variable declaration
- uint8_t *readData = nullptr, *tempMemory = nullptr;; // Pointer to store the read data
+ uint8_t readData[ECSS_MAX_STRING_SIZE]; // Preallocate the array
uint8_t memoryID = request.readEnum8(); // Read the memory ID from the request
// todo: Add checks depending on the memory type
@@ -68,23 +68,10 @@ void MemoryManagementService::RawDataMemoryManagement::dumpRawData(Message &requ
report.appendUint16(iterationCount); // Iteration count
// Iterate N times, as specified in the command message
- for (std::size_t j = 0, allocatedLength = 0; j < iterationCount; j++) {
+ for (std::size_t j = 0; j < iterationCount; j++) {
uint64_t startAddress = request.readUint64(); // Data length to read
uint16_t readLength = request.readUint16(); // Start address for the memory read
- // Allocate more array space if needed
- if (allocatedLength < readLength) {
- // todo: In embedded implementation use the malloc, due to FreeRTOS constraints
- tempMemory = static_cast<uint8_t *>(realloc(readData, readLength));
- if (tempMemory == nullptr) {
- // todo: Add error logging and reporting
- free(readData);
- } else {
- readData = tempMemory;
- allocatedLength = readLength;
- }
- }
-
// Read memory data, an octet at a time
for (std::size_t i = 0; i < readLength; i++) {
readData[i] = *(reinterpret_cast<uint8_t *>(startAddress) + i);
@@ -98,5 +85,4 @@ void MemoryManagementService::RawDataMemoryManagement::dumpRawData(Message &requ
mainService.storeMessage(report); // Save the report message
request.resetRead(); // Reset the reading count
- free(readData); // Free the allocated memory
}
diff --git a/test/Services/MemoryManagementService.cpp b/test/Services/MemoryManagementService.cpp
index 8fb43736..d59c8de2 100644
--- a/test/Services/MemoryManagementService.cpp
+++ b/test/Services/MemoryManagementService.cpp
@@ -32,7 +32,7 @@ TEST_CASE("TM[6,5]", "[service][st06]") {
uint8_t testString_2[8] = "SecStrT";
uint8_t testString_3[2] = {5, 8};
- uint8_t *checkString = nullptr;
+ uint8_t checkString[ECSS_MAX_STRING_SIZE];
uint16_t readSize = 0;
MemoryManagementService memMangService;
@@ -58,7 +58,7 @@ TEST_CASE("TM[6,5]", "[service][st06]") {
CHECK(response.readEnum8() == MemoryManagementService::MemoryID::RAM);
CHECK(response.readUint16() == 3);
CHECK(response.readUint64() == reinterpret_cast<uint64_t >(testString_1));
- checkString = response.readOctetString(&readSize);
+ readSize = response.readOctetString(checkString);
CHECK(readSize == sizeof(testString_1)/ sizeof(testString_1[0]));
CHECK(checkString[0] == 'F');
CHECK(checkString[1] == 'S');
@@ -68,7 +68,7 @@ TEST_CASE("TM[6,5]", "[service][st06]") {
CHECK(checkString[5] == '\0');
CHECK(response.readUint64() == reinterpret_cast<uint64_t >(testString_2));
- checkString = response.readOctetString(&readSize);
+ readSize = response.readOctetString(checkString);
CHECK(readSize == sizeof(testString_2)/ sizeof(testString_2[0]));
CHECK(checkString[0] == 'S');
CHECK(checkString[1] == 'e');
@@ -80,7 +80,7 @@ TEST_CASE("TM[6,5]", "[service][st06]") {
CHECK(checkString[7] == '\0');
CHECK(response.readUint64() == reinterpret_cast<uint64_t >(testString_3));
- checkString = response.readOctetString(&readSize);
+ readSize = response.readOctetString(checkString);
CHECK(readSize == sizeof(testString_3)/ sizeof(testString_3[0]));
CHECK(checkString[0] == 5);
CHECK(checkString[1] == 8);
--
GitLab