Fix memory issues #16
Conversation
github-actions
bot
added
the
DCCEXProtocol
| @@ -8,8 +8,8 @@ file(GLOB_RECURSE SRC src/*.cpp) | |||
| add_library(DCCEXProtocol STATIC ${SRC}) | |||
| add_library(DCCEX::Protocol ALIAS DCCEXProtocol) | |||
|
|
|||
| # Requires at least C++11 | |||
| target_compile_features(DCCEXProtocol PUBLIC cxx_std_11) | |||
| # Stuck at C++11 | |||
There was a problem hiding this comment.
Fixes the C++ standard to 11
| @@ -18,4 +18,4 @@ jobs: | |||
| CC: gcc-13 | |||
| CXX: g++-13 | |||
| - run: cmake --build build --parallel --target DCCEXProtocolTests | |||
| - run: ctest --test-dir build --schedule-random | |||
There was a problem hiding this comment.
ctest swallows errors thrown by LeakSanitizer, call test executable directly
|
|
||
| // Public methods | ||
|
|
||
| void DCCEXInbound::setup(int16_t maxParameterValues) { | ||
| _parameterValues = (int32_t *)malloc(maxParameterValues * sizeof(int32_t)); | ||
| _parameterValues = (int32_t *)realloc(_parameterValues, maxParameterValues * sizeof(int32_t)); |
There was a problem hiding this comment.
Better to realloc in case this function gets called multiple times
| @@ -62,6 +62,14 @@ DCCEXProtocol::DCCEXProtocol(int maxCmdBuffer) { | |||
| _bufflen = 0; | |||
| } | |||
|
|
|||
| DCCEXProtocol::~DCCEXProtocol() { | |||
There was a problem hiding this comment.
Add destructor to cleanup memory
src/DCCEXProtocol.cpp
Outdated
| versionStartAt = versionStartAt + strlen(temp) + 1; | ||
| temp = _nextServerDescriptionParam(description, versionStartAt, true); | ||
| _patchVersion = atoi(temp); | ||
| char *description{DCCEXInbound::getText(0) + 7}; |
There was a problem hiding this comment.
New "inplace" version parsing which does not create unnecessary copies
| } | ||
| _tempString[i] = '\0'; | ||
| char *_result; | ||
| _result = (char *)malloc(strlen(_tempString)); |
There was a problem hiding this comment.
This line caused an out of bounce access by the way
|
Suggestion for API-changes:
|
Ok, not sure if that's possible without way more effort. I will leave it like it is for now. |
As previously mentioned the DCCEXProtocol library has various memory issues (UB and leaks) which should be taken care of.
For now, let's turn on the sanitizer.