Skip to content

Commit

Permalink
Fix valgrind allocator mismatch warnings (#2766)
Browse files Browse the repository at this point in the history
This PR fixes warnings flagged by valgrind when running HostTests.

**Fix mismatched new/delete issues with SharedMemoryStream**

Use correct templated allocation, also simplifies code.

**Fix mismatched allocators with `LimitedMemoryStream`**

Use malloc/free` instead of new/delete, as for MemoryDataStream.
Resolves issue with mismatch highlighted by valgrind using `moveString`.
Also add null check in allocation.
  • Loading branch information
mikee47 authored Apr 15, 2024
1 parent cf1e532 commit 2c3ed1b
Show file tree
Hide file tree
Showing 5 changed files with 22 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -299,12 +299,14 @@ bool WebsocketConnection::send(IDataSourceStream* source, ws_frame_type_t type,

void WebsocketConnection::broadcast(const char* message, size_t length, ws_frame_type_t type)
{
char* copy = new char[length];
memcpy(copy, message, length);
std::shared_ptr<const char> data(copy, [](const char* ptr) { delete[] ptr; });
std::shared_ptr<char[]> data(new char[length]);
if(!data) {
return;
}
memcpy(data.get(), message, length);

for(auto skt : websocketList) {
auto stream = new SharedMemoryStream<const char>(data, length);
auto stream = new SharedMemoryStream<const char[]>(data, length);
skt->send(stream, type);
}
}
Expand Down
5 changes: 4 additions & 1 deletion Sming/Core/Data/Stream/LimitedMemoryStream.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,10 @@ int LimitedMemoryStream::seekFrom(int offset, SeekOrigin origin)
size_t LimitedMemoryStream::write(const uint8_t* data, size_t size)
{
if(buffer == nullptr) {
buffer = new char[capacity];
buffer = static_cast<char*>(malloc(capacity));
if(buffer == nullptr) {
return 0;
}
owned = true;
}

Expand Down
2 changes: 1 addition & 1 deletion Sming/Core/Data/Stream/LimitedMemoryStream.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ class LimitedMemoryStream : public ReadWriteStream
~LimitedMemoryStream()
{
if(owned) {
delete[] buffer;
free(buffer);
}
}

Expand Down
4 changes: 2 additions & 2 deletions Sming/Core/Data/Stream/SharedMemoryStream.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,9 @@ template <typename T> class SharedMemoryStream : public IDataSourceStream
public:
/** @brief Constructor for use with pre-existing buffer
* @param buffer
* @param capacity Size of buffer in elements
* @param size Size of buffer in elements
*/
SharedMemoryStream(std::shared_ptr<T>(buffer), size_t size) : buffer(buffer), capacity(size * sizeof(T))
SharedMemoryStream(std::shared_ptr<T>(buffer), size_t size) : buffer(buffer), capacity(size * sizeof(buffer[0]))
{
}

Expand Down
18 changes: 9 additions & 9 deletions tests/HostTests/modules/Stream.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -171,30 +171,30 @@ class StreamTest : public TestGroup

{
// STL may perform one-time memory allocation for mutexes, etc.
std::shared_ptr<const char> data(new char[18]);
SharedMemoryStream<const char>(data, 18);
std::shared_ptr<const char[]> data(new char[18]);
SharedMemoryStream stream(data, 18);
}

auto memStart = MallocCount::getCurrent();
// auto memStart = system_get_free_heap_size();

TEST_CASE("SharedMemoryStream")
{
char* message = new char[18];
memcpy(message, "Wonderful data...", 18);
std::shared_ptr<const char> data(message, [&message](const char* p) { delete[] p; });
const char* message = "Wonderful data...";
const size_t msglen = strlen(message);
std::shared_ptr<char[]> data(new char[msglen]);
memcpy(data.get(), message, msglen);

debug_d("RefCount: %d", data.use_count());

Vector<SharedMemoryStream<const char>*> list;
Vector<SharedMemoryStream<const char[]>*> list;
for(unsigned i = 0; i < 4; i++) {
list.addElement(new SharedMemoryStream<const char>(data, strlen(message)));
list.addElement(new SharedMemoryStream<const char[]>(data, msglen));
}

for(unsigned i = 0; i < list.count(); i++) {
for(auto element : list) {
constexpr size_t bufferSize{5};
char buffer[bufferSize]{};
auto element = list[i];

String output;
while(!element->isFinished()) {
Expand Down

0 comments on commit 2c3ed1b

Please sign in to comment.