CODEWITHSUNDEEP
c++memorymemory-leaks
Vishwanath Dalvi
Vishwanath Dalvi

Reputation: 36681

Why does this C++ program cause a memory leak?

Consider the following piece of code:

char* str1 = new char [30];    
char* str2 = new char [40];   

strcpy(str1, "Memory leak");    
str2 = str1;     

delete [] str2;     
delete [] str1;

Why does the above program cause a memory leak? How do I avoid this?

Upvotes: 5

Views: 1838

Answers (5)

Stefan
Stefan

Reputation: 4206

Because you're deleting str1 (the memory it points to) twice and you don't delete the memory allocated under where str2 was first pointing to.

EDIT: I'm not sure what you're trying to achieve.

char* str1 = new char [30];
// str1 = 0x00c06810; (i.e.)
char* str2 = new char [40];
// str2 = 0x00d12340; (i.e.)
strcpy(str1, "Memory leak");

// delete [] str2; should be here

str2 = str1; 
// now str2 == str1, so str2 = 0x00c06810 and str1 = 0x00c06810
// deleting 0x00c06810
delete [] str2; 
// deleting 0x00c06810 once again
delete [] str1;
// 0x00d12340 not deleted - memory leak

If you want that assignment (str2 = str1) then delete str2 first.

Upvotes: 20

Yuriy Vikulov
Yuriy Vikulov

Reputation: 2499

make pointers to be const pointers

The problem is that you allocate memory for two arrays, you get two pointers, then you overwrite address of the second array with address of first array. So you try to free memory of the first array

Upvotes: 2

templatetypedef
templatetypedef

Reputation: 373462

The above doesn't just lead to a memory leak; it causes undefined behavior, which is much, much worse.

The problem is in the last three lines:

str2 = str1; 
delete [] str2; 
delete [] str1;

If we ignore this first line, then the last two lines correctly reclaim all the memory allocated in this function. However, this first line sets str2 to point to the same buffer as str1. Since str2 is the only pointer in the program to the dynamic memory it references, this line leaks the memory for that buffer. Worse, when you then execute the next two lines to clean up the two pointers, you delete the same block of memory twice, once through str2 and once through str1. This leads to undefined behavior and often causes crashes. Particularly malicious users can actually use this to execute arbitrary code in your program, so be careful not to do this!

But there's one higher-level issue to take into account here. The whole problem with this setup is that you have to do all your own memory management. If you opt to use std::string instead of raw C-style strings, then you can write the code like this:

string str1 = "Memory leak"; // Actually, it doesn't. :-)
string str2;

str2 = str1; // Okay, make str2 a copy of str1

// All memory reclaimed when this function or block ends

Now, there's no need to explicitly manage the memory, and you don't have to worry about buffer overruns or double-frees. You're saved by the magic of object memory allocation.

Upvotes: 21

Yochai Timmer
Yochai Timmer

Reputation: 49269

Every object needs to be pointed-at in the memory.

The pointers keep track of where the data is (remember that your RAM memory is HUGE compared to the little array).

So in your case, you're losing the second array's place in memory. So there's an array lost somewhere in memory that you can't get to.

When you do str2 = str1; str2 points now to the block of memory that str1 pointed to. So there's nothing left to point at the second array.

Upvotes: 0

matekm
matekm

Reputation: 6030

You assign str1 pointer to str2 pointer, so delete[]str1 and delete[]str2 free only memory pointed by str1 (str2 points to the same memory). You need to free str2 memory before loosing pointer to it (before assigning str1 to str2)

The correct way is

char* str1 = new char [30];

char* str2 = new char [40]; //or just don't allocate this if You don;t need it

strcpy(str1, "Memory leak");

**delete [] str2;** 

str2 = str1; 


delete [] str1;

Upvotes: 4

Related Questions