From a81d59e3c0351ddd9e37a2d99eb1b9c1ad2d9628 Mon Sep 17 00:00:00 2001
From: Ioannis Kozaris <ioanniskozaris@gmail.com>
Date: Sat, 15 May 2021 11:26:18 +0000
Subject: [PATCH] Replace memcpy with std::copy. Closes #16
---
.idea/codeStyles/Project.xml | 3 +++
inc/Message.hpp | 6 +++---
src/Message.cpp | 22 ++++++----------------
src/MessageParser.cpp | 36 ++++++++++++------------------------
test/MessageParser.cpp | 10 ++++------
5 files changed, 28 insertions(+), 49 deletions(-)
diff --git a/.idea/codeStyles/Project.xml b/.idea/codeStyles/Project.xml
index 684c966e..e9c63b78 100644
--- a/.idea/codeStyles/Project.xml
+++ b/.idea/codeStyles/Project.xml
@@ -17,6 +17,9 @@
<pair source="c" header="h" fileNamingConvention="NONE" />
</extensions>
</Objective-C-extensions>
+ <clangFormatSettings>
+ <option name="ENABLED" value="true" />
+ </clangFormatSettings>
<codeStyleSettings language="ObjectiveC">
<option name="ALIGN_MULTILINE_CHAINED_METHODS" value="true" />
<option name="ALIGN_MULTILINE_FOR" value="false" />
diff --git a/inc/Message.hpp b/inc/Message.hpp
index f94c31be..b37d4ac2 100644
--- a/inc/Message.hpp
+++ b/inc/Message.hpp
@@ -104,7 +104,7 @@ public:
//
// @note This is initialized to 0 in order to prevent any mishaps with non-properly initialized values. \ref
// Message::appendBits() relies on this in order to easily OR the requested bits.
- uint8_t data[ECSS_MAX_MESSAGE_SIZE] = { 0 };
+ uint8_t data[ECSS_MAX_MESSAGE_SIZE] = {0};
// private:
uint8_t currentBit = 0;
@@ -222,7 +222,7 @@ public:
* ECSS_MAX_STRING_SIZE + 1. This function DOES place a \0 at the end of the created string,
* meaning that \p string should contain 1 more byte than the string stored in the message.
*/
- void readCString(char *string, uint16_t size);
+ void readCString(char* string, uint16_t size);
public:
Message(uint8_t serviceType, uint8_t messageType, PacketType packetType, uint16_t applicationId);
@@ -367,7 +367,7 @@ public:
* @param message The message to append
* @param size The fixed number of bytes that the message will take up. The empty last bytes are padded with 0s.
*/
- void appendMessage(const Message & message, uint16_t size);
+ void appendMessage(const Message& message, uint16_t size);
/**
* Fetches a single-byte boolean value from the current position in the message
diff --git a/src/Message.cpp b/src/Message.cpp
index 0333117e..21732cb9 100644
--- a/src/Message.cpp
+++ b/src/Message.cpp
@@ -6,7 +6,7 @@
#include <MessageParser.hpp>
Message::Message(uint8_t serviceType, uint8_t messageType, Message::PacketType packetType, uint16_t applicationId)
- : serviceType(serviceType), messageType(messageType), packetType(packetType), applicationId(applicationId) {}
+ : serviceType(serviceType), messageType(messageType), packetType(packetType), applicationId(applicationId) {}
void Message::appendBits(uint8_t numBits, uint16_t data) {
// TODO: Add assertion that data does not contain 1s outside of numBits bits
@@ -138,22 +138,18 @@ uint32_t Message::readWord() {
void Message::readString(char* string, uint16_t size) {
ASSERT_REQUEST((readPosition + size) <= ECSS_MAX_MESSAGE_SIZE, ErrorHandler::MessageTooShort);
ASSERT_REQUEST(size < ECSS_MAX_STRING_SIZE, ErrorHandler::StringTooShort);
-
- memcpy(string, data + readPosition, size);
-
+ std::copy(data + readPosition, data + readPosition + size, string);
readPosition += size;
}
void Message::readString(uint8_t* string, uint16_t size) {
ASSERT_REQUEST((readPosition + size) <= ECSS_MAX_MESSAGE_SIZE, ErrorHandler::MessageTooShort);
ASSERT_REQUEST(size < ECSS_MAX_STRING_SIZE, ErrorHandler::StringTooShort);
-
- memcpy(string, data + readPosition, size);
-
+ std::copy(data + readPosition, data + readPosition + size, string);
readPosition += size;
}
-void Message::readCString(char *string, uint16_t size) {
+void Message::readCString(char* string, uint16_t size) {
readString(string, size);
string[size] = 0;
}
@@ -171,20 +167,14 @@ void Message::appendString(const etl::istring& string) {
ASSERT_INTERNAL(dataSize + string.size() <= ECSS_MAX_MESSAGE_SIZE, ErrorHandler::MessageTooLarge);
// TODO: Do we need to keep this check? How does etl::string handle it?
ASSERT_INTERNAL(string.size() <= string.capacity(), ErrorHandler::StringTooLarge);
-
- memcpy(data + dataSize, string.data(), string.size());
-
+ std::copy(string.data(), string.data() + string.size(), data + dataSize);
dataSize += string.size();
}
void Message::appendFixedString(const etl::istring& string) {
ASSERT_INTERNAL((dataSize + string.max_size()) < ECSS_MAX_MESSAGE_SIZE, ErrorHandler::MessageTooLarge);
-
- // Append the bytes with content
- memcpy(data + dataSize, string.data(), string.size());
- // The rest of the bytes is set to 0
+ std::copy(string.data(), string.data() + string.size(), data + dataSize);
(void) memset(data + dataSize + string.size(), 0, string.max_size() - string.size());
-
dataSize += string.max_size();
}
diff --git a/src/MessageParser.cpp b/src/MessageParser.cpp
index 48f533b0..da379e2c 100644
--- a/src/MessageParser.cpp
+++ b/src/MessageParser.cpp
@@ -1,58 +1,48 @@
-#include <Services/EventActionService.hpp>
#include <ServicePool.hpp>
#include "ErrorHandler.hpp"
#include "MessageParser.hpp"
#include "macros.hpp"
-#include "Services/TestService.hpp"
#include "Services/RequestVerificationService.hpp"
#include "Helpers/CRCHelper.hpp"
void MessageParser::execute(Message& message) {
switch (message.serviceType) {
#ifdef SERVICE_EVENTREPORT
- case 5:
- Services.eventReport.execute(message); // ST[05]
+ case 5: Services.eventReport.execute(message); // ST[05]
break;
#endif
#ifdef SERVICE_MEMORY
- case 6:
- Services.memoryManagement.execute(message); // ST[06]
+ case 6: Services.memoryManagement.execute(message); // ST[06]
break;
#endif
#ifdef SERVICE_FUNCTION
- case 8:
- Services.functionManagement.execute(message); // ST[08]
+ case 8: Services.functionManagement.execute(message); // ST[08]
break;
#endif
#ifdef SERVICE_TIMESCHEDULING
- case 11:
- Services.timeBasedScheduling.execute(message); // ST[11]
+ case 11: Services.timeBasedScheduling.execute(message); // ST[11]
break;
#endif
#ifdef SERVICE_TEST
- case 17:
- Services.testService.execute(message); // ST[17]
+ case 17: Services.testService.execute(message); // ST[17]
break;
#endif
#ifdef SERVICE_EVENTACTION
- case 19:
- Services.eventAction.execute(message); // ST[19]
+ case 19: Services.eventAction.execute(message); // ST[19]
break;
#endif
#ifdef SERVICE_PARAMETER
- case 20:
- Services.parameterManagement.execute(message); // ST[20]
+ case 20: Services.parameterManagement.execute(message); // ST[20]
break;
#endif
- default:
- ErrorHandler::reportInternalError(ErrorHandler::OtherMessageType);
+ default: ErrorHandler::reportInternalError(ErrorHandler::OtherMessageType);
}
}
@@ -69,7 +59,7 @@ Message MessageParser::parse(uint8_t* data, uint32_t length) {
bool secondaryHeaderFlag = (data[0] & 0x08U) != 0U;
uint16_t APID = packetHeaderIdentification & static_cast<uint16_t>(0x07ff);
auto sequenceFlags = static_cast<uint8_t>(packetSequenceControl >> 14);
- uint16_t packetSequenceCount = packetSequenceControl & (~ 0xc000U); // keep last 14 bits
+ uint16_t packetSequenceCount = packetSequenceControl & (~0xc000U); // keep last 14 bits
// Returning an internal error, since the Message is not available yet
ASSERT_INTERNAL(versionNumber == 0U, ErrorHandler::UnacceptablePacket);
@@ -103,10 +93,9 @@ void MessageParser::parseECSSTCHeader(const uint8_t* data, uint16_t length, Mess
length -= 5;
// Copy the data to the message
- // TODO: See if memcpy is needed for this
message.serviceType = serviceType;
message.messageType = messageType;
- memcpy(message.data, data + 5, length);
+ std::copy(data + 5, data + 5 + length, message.data);
message.dataSize = length;
}
@@ -192,7 +181,7 @@ String<CCSDS_MAX_MESSAGE_SIZE> MessageParser::compose(const Message& message) {
#if ECSS_CRC_INCLUDED
// Append CRC field
uint16_t crcField = CRCHelper::calculateCRC(reinterpret_cast<uint8_t*>(ccsdsMessage.data()), 6 +
- packetDataLength);
+ packetDataLength);
ccsdsMessage.push_back(static_cast<uint8_t>(crcField >> 8U));
ccsdsMessage.push_back(static_cast<uint8_t>(crcField & 0xFF));
#endif
@@ -215,9 +204,8 @@ void MessageParser::parseECSSTMHeader(const uint8_t* data, uint16_t length, Mess
length -= 5;
// Copy the data to the message
- // TODO: See if memcpy is needed for this
message.serviceType = serviceType;
message.messageType = messageType;
- memcpy(message.data, data + 5, length);
+ std::copy(data + 5, data + 5 + length, message.data);
message.dataSize = length;
}
diff --git a/test/MessageParser.cpp b/test/MessageParser.cpp
index c2e8981b..e840b990 100644
--- a/test/MessageParser.cpp
+++ b/test/MessageParser.cpp
@@ -1,12 +1,8 @@
#include <catch2/catch.hpp>
-#include <Services/TestService.hpp>
-#include <Services/RequestVerificationService.hpp>
#include <Message.hpp>
#include <cstring>
#include "Helpers/CRCHelper.hpp"
#include "MessageParser.hpp"
-#include "Services/ServiceTests.hpp"
-#include "ServicePool.hpp"
TEST_CASE("TC message parsing", "[MessageParser]") {
@@ -32,7 +28,8 @@ TEST_CASE("TC Message parsing into a string", "[MessageParser]") {
message.serviceType = 129;
message.messageType = 31;
message.packetSequenceCount = 8199;
- memcpy(message.data, "hello", 5);
+ String<5> sourceString = "hello";
+ std::copy(sourceString.data(), sourceString.data() + sourceString.size(), message.data);
message.dataSize = 5;
String<CCSDS_MAX_MESSAGE_SIZE> createdPacket = MessageParser::compose(message);
@@ -73,7 +70,8 @@ TEST_CASE("TM Message parsing into a string", "[MessageParser]") {
message.packetSequenceCount = 77;
message.serviceType = 22;
message.messageType = 17;
- memcpy(message.data, "hellohi", 7);
+ String<7> sourceString = "hellohi";
+ std::copy(sourceString.data(), sourceString.data() + sourceString.size(), message.data);
message.dataSize = 7;
String<CCSDS_MAX_MESSAGE_SIZE> createdPacket = MessageParser::compose(message);
--
GitLab