0

I'm investigating a memory leak issue that we have in our code. There is the following struct:

struct Config {
    const uint32_t version = 1;
    const uint32_t _sizeof = (5*sizeof(uint32_t));
    uint32_t lengthOriginal;
    uint32_t lengthCompressed;
    uint32_t reserved = 0;
    unsigned char buffer[1];
};

It is used with the unique_ptr in the following manner:

static Config inited;
std::unique_ptr<Config> ptr;
ptr.reset(reinterpret_cast<Config *>(new char[1 + inited._sizeof + dataLength]));
memset(ptr.get(), 0, inited._sizeof + 100);
memcpy(ptr.get(), &inited, sizeof(inited));

And I'm pretty sure that the memory leak comes from mixing the unique_ptr and the new operator.
When running the Valgrind Memcheck, it warns about Mismatched free() / delete / delete [], but it shows All heap blocks were freed -- no leaks are possible.

On the other side, trying to run this code with overridden operator delete ( void* ptr, std::size_t sz ), the size it tries to release is 24 bytes, which is much less than the allocated memory size.

Currently, I'm not sure if there is a memory leak here?
And if so, how to solve it? Possibly, passing a custom deleter to the unique pointer will solve this.
But, the other question is it a common practice to use memory allocation and smart pointers in that way?
The struct itself cannot be changed.

Digor
  • 36
  • 1
  • 2
  • 2
    There's undefined behaviour here. You don't construct a `Config` object but you do destroy one. – john Jul 17 '22 at 12:51
  • What does it say if you replace `new char[N]` with `operator new(N)`? This looks like UB (destroying a wrong type; and also going out of bounds on `buffer`), but I'd expect the deletion to work in practice, especially after the proposed change. I would also remove the `[1]` array completely, and calculate the element location manually (probably with a member function), to get rid of the UB. – HolyBlackCat Jul 17 '22 at 12:51
  • 2
    You are trying to use `unsigned char buffer[1];` as a _flexible array member_, which don't exist in C++. Doing this in C++ causes undefined behavior and even in C where it is allowed, the array must have empty bounds. – user17732522 Jul 17 '22 at 12:52
  • There are many problems with the code as shown, like the assumption that a structure size is equal to the size of its members, [which is false](https://stackoverflow.com/questions/119123/why-isnt-sizeof-for-a-struct-equal-to-the-sum-of-sizeof-of-each-member). – Some programmer dude Jul 17 '22 at 12:55
  • Also, why don't you make the `version` and `_sizeof` members `static`? Then they won't actually be part of the structure instances and can save you a few bytes if you need multiple instances. – Some programmer dude Jul 17 '22 at 12:56
  • I don't understand why you'd blame the `unique_ptr` and `new` while completely ignoring the `reinterpret_cast` middleman. Rule of thumb is that `reinterpret_cast` is to be avoided, and if something goes wrong, you now know why it's best avoided. – StoryTeller - Unslander Monica Jul 17 '22 at 13:12
  • 3
    You'll need a customer deleter. The allocated `new[]` of a `char[]` will be destructed with a `delete` of a `Config*`. And that's not compatible. (On my system, the `new[]` array heap is separate from the `new` non-array heap, and is separate from the `malloc` heap.) – Eljay Jul 17 '22 at 13:23
  • What is the *actual* and *underlying* problem you try to solve here? You have shown us a solution, but it's to an (for us) unknown problem. We can say that the solution is bad, and in this case wrong no matter the underlying problem. But if you want our help to solve your original problem then please ask about that *directly*. Right now this question is an [XY problem](https://xyproblem.info/) – Some programmer dude Jul 17 '22 at 14:39
  • @Someprogrammerdude As I wrote at the beginning of my question, I'm investigating a memory leak. This code snippet is what I found. This code is not a solution. Our components widely use it to compress and transfer data over network channels. The question was if it could cause a memory leak. Since some of the comments were helpful, I got the answer that I looked for. – Digor Jul 17 '22 at 14:54

0 Answers0