added improvements from code review

This commit is contained in:
Robin Müller 2021-09-11 19:18:18 +02:00
parent ea573b0523
commit 7c7a8a5df7
No known key found for this signature in database
GPG Key ID: 11D4952C8CCEF814
3 changed files with 65 additions and 35 deletions

View File

@ -8,22 +8,12 @@ DleEncoder::~DleEncoder() {}
ReturnValue_t DleEncoder::encode(const uint8_t* sourceStream, ReturnValue_t DleEncoder::encode(const uint8_t* sourceStream,
size_t sourceLen, uint8_t* destStream, size_t maxDestLen, size_t sourceLen, uint8_t* destStream, size_t maxDestLen,
size_t* encodedLen, bool addStxEtx) { size_t* encodedLen, bool addStxEtx) {
size_t minAllowedLen = 0;
if(escapeStxEtx) {
minAllowedLen = 1;
}
else {
minAllowedLen = 2;
}
if(minAllowedLen > maxDestLen) {
return STREAM_TOO_SHORT;
}
if (addStxEtx) { if (addStxEtx) {
size_t currentIdx = 0; size_t currentIdx = 0;
if(not escapeStxEtx) { if(not escapeStxEtx) {
destStream[currentIdx++] = DLE_CHAR; destStream[currentIdx++] = DLE_CHAR;
} }
destStream[currentIdx] = STX_CHAR;
} }
if(escapeStxEtx) { if(escapeStxEtx) {
@ -40,9 +30,15 @@ ReturnValue_t DleEncoder::encode(const uint8_t* sourceStream,
ReturnValue_t DleEncoder::encodeStreamEscaped(const uint8_t *sourceStream, size_t sourceLen, ReturnValue_t DleEncoder::encodeStreamEscaped(const uint8_t *sourceStream, size_t sourceLen,
uint8_t *destStream, size_t maxDestLen, size_t *encodedLen, uint8_t *destStream, size_t maxDestLen, size_t *encodedLen,
bool addStxEtx) { bool addStxEtx) {
size_t encodedIndex = 1; size_t encodedIndex = 0;
size_t sourceIndex = 0; size_t sourceIndex = 0;
uint8_t nextByte = 0; uint8_t nextByte = 0;
if(addStxEtx) {
if(maxDestLen < 1) {
return STREAM_TOO_SHORT;
}
destStream[encodedIndex++] = STX_CHAR;
}
while (encodedIndex < maxDestLen and sourceIndex < sourceLen) { while (encodedIndex < maxDestLen and sourceIndex < sourceLen) {
nextByte = sourceStream[sourceIndex]; nextByte = sourceStream[sourceIndex];
// STX, ETX and CR characters in the stream need to be escaped with DLE // STX, ETX and CR characters in the stream need to be escaped with DLE
@ -82,8 +78,11 @@ ReturnValue_t DleEncoder::encodeStreamEscaped(const uint8_t *sourceStream, size_
++sourceIndex; ++sourceIndex;
} }
if (sourceIndex == sourceLen and encodedIndex < maxDestLen) { if (sourceIndex == sourceLen) {
if (addStxEtx) { if (addStxEtx) {
if(encodedIndex + 1 >= maxDestLen) {
return STREAM_TOO_SHORT;
}
destStream[encodedIndex] = ETX_CHAR; destStream[encodedIndex] = ETX_CHAR;
++encodedIndex; ++encodedIndex;
} }
@ -98,9 +97,16 @@ ReturnValue_t DleEncoder::encodeStreamEscaped(const uint8_t *sourceStream, size_
ReturnValue_t DleEncoder::encodeStreamNonEscaped(const uint8_t *sourceStream, size_t sourceLen, ReturnValue_t DleEncoder::encodeStreamNonEscaped(const uint8_t *sourceStream, size_t sourceLen,
uint8_t *destStream, size_t maxDestLen, size_t *encodedLen, uint8_t *destStream, size_t maxDestLen, size_t *encodedLen,
bool addStxEtx) { bool addStxEtx) {
size_t encodedIndex = 2; size_t encodedIndex = 0;
size_t sourceIndex = 0; size_t sourceIndex = 0;
uint8_t nextByte = 0; uint8_t nextByte = 0;
if(addStxEtx) {
if(maxDestLen < 2) {
return STREAM_TOO_SHORT;
}
destStream[encodedIndex++] = DLE_CHAR;
destStream[encodedIndex++] = STX_CHAR;
}
while (encodedIndex < maxDestLen and sourceIndex < sourceLen) { while (encodedIndex < maxDestLen and sourceIndex < sourceLen) {
nextByte = sourceStream[sourceIndex]; nextByte = sourceStream[sourceIndex];
// DLE characters are simply escaped with DLE. // DLE characters are simply escaped with DLE.
@ -121,7 +127,7 @@ ReturnValue_t DleEncoder::encodeStreamNonEscaped(const uint8_t *sourceStream, si
++sourceIndex; ++sourceIndex;
} }
if (sourceIndex == sourceLen and encodedIndex < maxDestLen) { if (sourceIndex == sourceLen) {
if (addStxEtx) { if (addStxEtx) {
if(encodedIndex + 2 >= maxDestLen) { if(encodedIndex + 2 >= maxDestLen) {
return STREAM_TOO_SHORT; return STREAM_TOO_SHORT;
@ -140,17 +146,6 @@ ReturnValue_t DleEncoder::encodeStreamNonEscaped(const uint8_t *sourceStream, si
ReturnValue_t DleEncoder::decode(const uint8_t *sourceStream, ReturnValue_t DleEncoder::decode(const uint8_t *sourceStream,
size_t sourceStreamLen, size_t *readLen, uint8_t *destStream, size_t sourceStreamLen, size_t *readLen, uint8_t *destStream,
size_t maxDestStreamlen, size_t *decodedLen) { size_t maxDestStreamlen, size_t *decodedLen) {
size_t encodedIndex = 0;
if(not escapeStxEtx) {
if (*sourceStream != DLE_CHAR) {
return DECODING_ERROR;
}
++encodedIndex;
}
if (sourceStream[encodedIndex] != STX_CHAR) {
return DECODING_ERROR;
}
if(escapeStxEtx) { if(escapeStxEtx) {
return decodeStreamEscaped(sourceStream, sourceStreamLen, return decodeStreamEscaped(sourceStream, sourceStreamLen,
readLen, destStream, maxDestStreamlen, decodedLen); readLen, destStream, maxDestStreamlen, decodedLen);
@ -164,10 +159,15 @@ ReturnValue_t DleEncoder::decode(const uint8_t *sourceStream,
ReturnValue_t DleEncoder::decodeStreamEscaped(const uint8_t *sourceStream, size_t sourceStreamLen, ReturnValue_t DleEncoder::decodeStreamEscaped(const uint8_t *sourceStream, size_t sourceStreamLen,
size_t *readLen, uint8_t *destStream, size_t *readLen, uint8_t *destStream,
size_t maxDestStreamlen, size_t *decodedLen) { size_t maxDestStreamlen, size_t *decodedLen) {
// Skip start marker, was already checked size_t encodedIndex = 0;
size_t encodedIndex = 1;
size_t decodedIndex = 0; size_t decodedIndex = 0;
uint8_t nextByte; uint8_t nextByte;
if(maxDestStreamlen < 1) {
return STREAM_TOO_SHORT;
}
if (sourceStream[encodedIndex++] != STX_CHAR) {
return DECODING_ERROR;
}
while ((encodedIndex < sourceStreamLen) while ((encodedIndex < sourceStreamLen)
and (decodedIndex < maxDestStreamlen) and (decodedIndex < maxDestStreamlen)
and (sourceStream[encodedIndex] != ETX_CHAR) and (sourceStream[encodedIndex] != ETX_CHAR)
@ -192,6 +192,8 @@ ReturnValue_t DleEncoder::decodeStreamEscaped(const uint8_t *sourceStream, size_
destStream[decodedIndex] = nextByte - 0x40; destStream[decodedIndex] = nextByte - 0x40;
} }
else { else {
// Set readLen so user can resume parsing after incorrect data
*readLen = encodedIndex + 2;
return DECODING_ERROR; return DECODING_ERROR;
} }
} }
@ -205,8 +207,13 @@ ReturnValue_t DleEncoder::decodeStreamEscaped(const uint8_t *sourceStream, size_
++decodedIndex; ++decodedIndex;
} }
if (sourceStream[encodedIndex] != ETX_CHAR) { if (sourceStream[encodedIndex] != ETX_CHAR) {
*readLen = ++encodedIndex; if(decodedIndex == maxDestStreamlen) {
return DECODING_ERROR; return STREAM_TOO_SHORT;
}
else {
*readLen = ++encodedIndex;
return DECODING_ERROR;
}
} }
else { else {
*readLen = ++encodedIndex; *readLen = ++encodedIndex;
@ -218,18 +225,29 @@ ReturnValue_t DleEncoder::decodeStreamEscaped(const uint8_t *sourceStream, size_
ReturnValue_t DleEncoder::decodeStreamNonEscaped(const uint8_t *sourceStream, ReturnValue_t DleEncoder::decodeStreamNonEscaped(const uint8_t *sourceStream,
size_t sourceStreamLen, size_t *readLen, uint8_t *destStream, size_t sourceStreamLen, size_t *readLen, uint8_t *destStream,
size_t maxDestStreamlen, size_t *decodedLen) { size_t maxDestStreamlen, size_t *decodedLen) {
// Skip start marker, was already checked size_t encodedIndex = 0;
size_t encodedIndex = 2;
size_t decodedIndex = 0; size_t decodedIndex = 0;
uint8_t nextByte; uint8_t nextByte;
if(maxDestStreamlen < 2) {
return STREAM_TOO_SHORT;
}
if (sourceStream[encodedIndex++] != DLE_CHAR) {
return DECODING_ERROR;
}
if (sourceStream[encodedIndex++] != STX_CHAR) {
return DECODING_ERROR;
}
while ((encodedIndex < sourceStreamLen) && (decodedIndex < maxDestStreamlen)) { while ((encodedIndex < sourceStreamLen) && (decodedIndex < maxDestStreamlen)) {
if (sourceStream[encodedIndex] == DLE_CHAR) { if (sourceStream[encodedIndex] == DLE_CHAR) {
if(encodedIndex + 1 >= sourceStreamLen) { if(encodedIndex + 1 >= sourceStreamLen) {
*readLen = encodedIndex;
return DECODING_ERROR; return DECODING_ERROR;
} }
nextByte = sourceStream[encodedIndex + 1]; nextByte = sourceStream[encodedIndex + 1];
if(nextByte == STX_CHAR) { if(nextByte == STX_CHAR) {
*readLen = ++encodedIndex; // Set readLen so the DLE/STX char combination is preserved. Could be start of
// another frame
*readLen = encodedIndex;
return DECODING_ERROR; return DECODING_ERROR;
} }
else if(nextByte == DLE_CHAR) { else if(nextByte == DLE_CHAR) {
@ -245,6 +263,7 @@ ReturnValue_t DleEncoder::decodeStreamNonEscaped(const uint8_t *sourceStream,
return RETURN_OK; return RETURN_OK;
} }
else { else {
*readLen = encodedIndex;
return DECODING_ERROR; return DECODING_ERROR;
} }
} }
@ -254,7 +273,13 @@ ReturnValue_t DleEncoder::decodeStreamNonEscaped(const uint8_t *sourceStream,
++encodedIndex; ++encodedIndex;
++decodedIndex; ++decodedIndex;
} }
return DECODING_ERROR; *readLen = encodedIndex;
if(decodedIndex == maxDestStreamlen) {
return STREAM_TOO_SHORT;
}
else {
return DECODING_ERROR;
}
} }
void DleEncoder::setEscapeMode(bool escapeStxEtx) { void DleEncoder::setEscapeMode(bool escapeStxEtx) {

View File

@ -71,6 +71,8 @@ public:
* @param addStxEtx Adding STX start marker and ETX end marker can be omitted, * @param addStxEtx Adding STX start marker and ETX end marker can be omitted,
* if they are added manually * if they are added manually
* @return * @return
* - RETURN_OK for successful encoding operation
* - STREAM_TOO_SHORT if the destination stream is too short
*/ */
ReturnValue_t encode(const uint8_t *sourceStream, size_t sourceLen, ReturnValue_t encode(const uint8_t *sourceStream, size_t sourceLen,
uint8_t *destStream, size_t maxDestLen, size_t *encodedLen, uint8_t *destStream, size_t maxDestLen, size_t *encodedLen,
@ -85,6 +87,9 @@ public:
* @param maxDestStreamlen * @param maxDestStreamlen
* @param decodedLen * @param decodedLen
* @return * @return
* - RETURN_OK for successful decode operation
* - DECODE_ERROR if the source stream is invalid
* - STREAM_TOO_SHORT if the destination stream is too short
*/ */
ReturnValue_t decode(const uint8_t *sourceStream, ReturnValue_t decode(const uint8_t *sourceStream,
size_t sourceStreamLen, size_t *readLen, uint8_t *destStream, size_t sourceStreamLen, size_t *readLen, uint8_t *destStream,

View File

@ -165,7 +165,7 @@ TEST_CASE("DleEncoder" , "[DleEncoder]") {
result = dleEncoder.decode(vecToDecode.data(), result = dleEncoder.decode(vecToDecode.data(),
vecToDecode.size(), &readLen, vecToDecode.size(), &readLen,
buffer.data(), faultyDestSizes, &decodedLen); buffer.data(), faultyDestSizes, &decodedLen);
REQUIRE(result == static_cast<int>(DleEncoder::DECODING_ERROR)); REQUIRE(result == static_cast<int>(DleEncoder::STREAM_TOO_SHORT));
} }
}; };