C++ Memory leak when freeing *?

Question

I am using this in a void:

unsigned char *nEncodedBytes = NULL;
nEncodedBytes = new unsigned char[m_lLenCompressedBytes];

short *shrtDecoded = NULL;
shrtDecoded = new short[iCountDecodedShorts];

short *m_ShortOut;
m_ShortOut = (short*)malloc(( 960*6)*sizeof(short));

unsigned char *m_DataBytes;
m_DataBytes = (unsigned char*)calloc((960*6),sizeof(char));

When I am done, I free the memory using

delete (nEncodedBytes);
delete (shrtDecoded);

    free(m_DataBytes);
    free(m_ShortOut);

Is this fine? I am unsure why I was using delete in one place, and free in the other. I copied my code around.

Is there a memory leak?

Thank you.

You can do a `valgrind` and test it yourself. Plus I believe mixing `new` paradigm with `malloc` results in undefined behaviour, which kinda would end any discussion for most people. — luk32, Oct 09 '13 at 15:46
If you have access to Linux then I strongly encourage you to try out [valgrind](http://valgrind.org/). It really is a miracle tool for weeding out memory leaks/problems in C/C++ programs. — PP., Oct 09 '13 at 15:47
@PP. Actually `clang` does a much better job since it can instrument the code at the compilation stage. — syam, Oct 09 '13 at 15:51
@luk32: Mixing `new` and `free` in a C++ program does not in itself invoke UB. That doesn't make it a good idea. — John Dibling, Oct 09 '13 at 15:58
@JohnDibling I stand corrected. Thank you. I know `new` can be overloaded and invokes constructor which is pretty important when you are accutally using `c++` as a language, not just a compiler. But I did think the memory allocators would mess each other, which is not true. It kinda looked to me like using two standard libraries =). — luk32, Oct 09 '13 at 16:09

score 4 · Accepted Answer · answered Oct 09 '13 at 15:49

You use free when you use malloc. In all likelihood, you shouldn't be using malloc at all in C++; it's the C way of doing things and only rarely wanted in C++.

You use delete when you allocate with new. new calls the constructor as well as allocating memory, and delete calls the destructor as well as releasing memory. These are thus the object orientated C++ options. There is, however, a wrinkle. Because the implementation of C++ doesn't know whether a pointer refers to an array or a single object if you allocate an array (e.g. nEncodedBytes = new unsigned char[m_lLenCompressedBytes];) then you should use delete[] instead of delete to release it.

Mind you, failing to call delete[] only means that you will only call the destructor for the first object in an array so, in this particular case there should be no difference in the outcome between calling delete[] and calling delete because char has no destructor.

I don't see a memory leak in your code but since you haven't posted all your code we can't tell.

Using delete on a pointer allocated with new[] is undefined behaviour. The idea that it only calls one destructor may be true of one particular compiler but it's not guaranteed by any standard. — john, Oct 09 '13 at 16:19

score 2 · Answer 2 · answered Oct 09 '13 at 15:45

2

You should use

delete [] nEncodedBytes;
delete [] shrtDecoded;

as you are deleting arrays.

answered Oct 09 '13 at 15:45

hivert

10,579
3
31
56

While this is true it doesn't make a difference to the question asked about a memory leak or answer the question about `free` vs `delete` – Jack Aidley Oct 09 '13 at 15:51
@JackAidley This could be argued. Calling `delete` on `new(]`'ed arrays is UB and might well lead to leaks (it *does* on a number of implementations). But sure the answer could be improved. – syam Oct 09 '13 at 15:53
@syam: You _definitely_ shouldn't do it, that's for sure, but it will release all the memory when dealing with a `char` array because a `char` has no destructor that would release further memory. – Jack Aidley Oct 09 '13 at 15:56
3

@JackAidley: It might release all the memory; or it might not, depending on how the free store is implemented. All we know for sure is that you get undefined behaviour. – Mike Seymour Oct 09 '13 at 15:58
1

@JackAidley Even for `char` arrays there is absolutely no guarantee that `delete` will correctly delete the whole array. Again, some C++ implementations are known to segfault in this case, others will silently leak, and others will just do the right thing. Which is UB in all its glory (including *appearing* to work, which is what bit you here). – syam Oct 09 '13 at 16:10

score 2 · Answer 3 · edited May 23 '17 at 12:28

2

Don't mix malloc and new (first because of stylistic reasons, and then because you should never delete a malloc-ed memory or free a new-ed zone). Consider using standard C++ containers. You won't even need to explicitly allocate memory (the library will do that for you). You could code

std::vector<char> nEncodedBytes;
nEncodedBytes.resize(encodedbyteslen);

On Linux, use valgrind to hunt memory leaks. BTW, you might be interested by Boehm's GC, perhaps using its allocator like here.

BTW, when using yourself malloc you should always test its result, at least like

 SomeType* ptr = malloc(sizeof(SomeType));
 if (!ptr) { perror("malloc SomeType"); exit(EXIT_FAILURE); };

remember that malloc could fail. You may want to limit the memory available (e.g. with ulimit -m in bash in your terminal) for testing purposes (e.g. to make malloc -or new- fail more easily to ensure you handle that kind of failure well enough).

edited May 23 '17 at 12:28

Community

1
1

answered Oct 09 '13 at 15:45

Basile Starynkevitch

223,805
18
296
547

2

Apart from consistency reasons there's no technical reason why `malloc` and `new` couldn't be mixed. So long as `malloc`s are matched with `free`s, `new`s are matched with `delete`s, and `new[]`s are matched with `delete[]`s. – PP. Oct 09 '13 at 15:48
I used vectorshrtDecoded; shrtDecoded.resize(iCountDecodedShorts+1); now, but I need to pass them to a function that expects "short *". Can I still use vector as an argument? I tried passing shrtDecoded[0], but that wouldn't work. – tmighty Oct 09 '13 at 15:51
1

That's because `shrtDecoded[0]` isn't a *pointer* to a short, Tmighty. Use the `&` operator to get a pointer to it, and you'll be fine. In newer version of C++, you can use `shrtDecoded.data()`. – Rob Kennedy Oct 09 '13 at 15:53
Some people would probably kill me for saying this, but you can do `short * pshrtDecoded = &shrtDecoded[0]` and pass that. As long as you're not doing anything fancy with allocators, the [] operator returns a reference to the first element. Just don't try to delete that pointer (vector will do it for you) – shash Oct 09 '13 at 16:01

score 0 · Answer 4 · answered Oct 09 '13 at 16:12

Is this fine?

Not quite. There are a couple problems. One -- your troublesome use of free -- we'll address in a moment. But first, your use of new[] with delete (non-[]) invokes Undefined Behavior:

Here you are using new []:

nEncodedBytes = new unsigned char[m_lLenCompressedBytes];

And here you are using delete:

delete (nEncodedBytes);

This should be:

delete [] nEncodedBytes;

Using the [] for of new with the non-[] form of delete invokes Undefined Behavior. Now in reality, all the compilers & platforms I'm aware of will handle this fine and do what you expect in this particular case -- but you should never rely on Undefined Behavior.

Now for your use of malloc and free:

I am unsure why I was using delete in one place, and free in the other.

You probably shouldn't be. Here's a rule of thumb:

In C++, always use new and delete; never use malloc and free.

Yes, there are exceptions, but they are first of all rare, and second of all you will know exactly when the exceptions come in to play. In the example you've posted here, I see no reason that compels the use of malloc and free. Hence, you should not be.

Contrary to (popular?) belief, mixing new/delete and malloc/free in a single C++ program does no in itself invoke Undefined Behavior or make your program otherwise ill-formed. You can do it, if you do it properly. But you still shouldn't.

Is there a memory leak?

Well, since you have invoked Undefined Behavior, there could be. Undefined Behavior means anything can happen. You are however de-allocating everything you allocate in the code shown. So aside from the UB, I see no memory leak here.

C++ Memory leak when freeing *?

4 Answers4