Memory leaks within a class constructor

closed account (2AoiNwbp)
Hi, I am a begginer dealing (trying to understand actually) dynamic memory. I took the following code from a book and try to see how it works, and I get memory leaks (using visual leak detector) when executing the following code:

(there's a "char* pmessage" private member in the class definition, deleted in the class destructor)

1
2
3
4
5
6
7
// Constructor definition
CMessage(const char* text = "Default message")
{
  cout < < "Constructor called." < < endl;
  pmessage = new char[strlen(text) + 1]; // Allocate space for text
  strcpy_s(pmessage, strlen(text)+1, text); // Copy text to new memory
}


But, if I change the code as follows, memory leaks disappear:

1
2
3
4
5
6
7
8
9
10
11
12
// Constructor definition
CMessage(const char* text = NULL)
{
cout << "Constructor called." << endl;
  if(!text)
    pmessage = NULL;
  else 
  {
    pmessage = new char[strlen(text) + 1]; // Allocate space for ext
    strcpy_s(pmessage, strlen(text)+1, text); // Copy text to new memory
  }
}


My question is: why is it happening??
As far as I understand, there's no memory allocation (using the new op) in the function (constructor) call. So I don't get it.
Please, help.
Thank you!

Alejandro
I don't know why you get a memory leak. It depends on the rest of the code.

In your second code, if text is always null then there will of course be no memory leak caused by this code because there is no dynamic memory allocation.
closed account (j3Rz8vqX)
Like Peter87 stated, the memory leak is dependent elsewhere.

Your second code "may" never implement its else, possibly avoiding the creation of the new char array.

Memory leak only occurs if you've not deleted the memory before the end of the program executes.

Ensure you've deleted all new values.

If you're reallocating memory, keep an eye on each memory and ensure it is deleted when no longer used.

Make sure each memory has something pointing to it, or know the locations of your memory so you can later physically delete them.
Last edited on
closed account (2AoiNwbp)
Hi guys,

In the complete code below, there are two objects (motto1 and motto2 - lines 96 and 97) initialized with a text string, using the "const char* constructor" shown above. If I set it to "Default Message", I get memory leaks, but, if I set it to NULL, there are no memory leaks (Output messages after code). Thus, where is the memory allocation? since no "new" operator is used inside this constructor?

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
#include <iostream>
#include <cstring>
#include <vld.h>

using std::cout;
using std::endl;

class CMessage
{
private:
	char* pmessage; // Pointer to object text string
public:
	// Function to display a message
	void ShowIt() const
	{
		cout << endl << pmessage;
	}
	// Overloaded addition operator
	CMessage operator+(const CMessage & aMess) const
	{
		cout << "Add operator function called." << endl;
		size_t len = strlen(pmessage) + strlen(aMess.pmessage) + 1;
		CMessage message;
		message.pmessage = new char[len];
		strcpy_s(message.pmessage, len, pmessage);
		strcat_s(message.pmessage, len, aMess.pmessage);
		return message;
	}
	// Overloaded assignment operator for CMessage objects
	CMessage & operator=(const CMessage & aMess)
	{
		cout << "Assignment operator function called." << endl;
		if(this == & aMess) // Check addresses, if equal
			return *this; // return the 1st operand
		// Release memory for 1st operand
		delete[] pmessage;
		pmessage = new char[strlen(aMess.pmessage) + 1];
		// Copy 2nd operand string to 1st
		strcpy_s(this->pmessage, strlen(aMess.pmessage)+1, aMess.pmessage);
		// Return a reference to 1st operand
		return *this;
	}
	
	CMessage & operator=(CMessage && aMess)
	{
		cout << "Move assignment operator function called." << endl;
		delete[] pmessage; // Release memory for left operand
		pmessage = aMess.pmessage; // Steal string from rhs object
		aMess.pmessage = nullptr; // Null rhs pointer
		return *this; // Return a reference to 1st operand
	}

	// Constructor definition
	CMessage(const char* text = "Default Message") // *** ERROR *** //
	{
		cout << "const char* constructor called." << endl;
		if(!text)
			pmessage = NULL;
		else 
		{
			pmessage = new char[strlen(text) + 1]; // Allocate space for text
			strcpy_s(pmessage, strlen(text)+1, text); // Copy text to new memory
		}
	}
	// Copy constructor definition
	CMessage(const CMessage & aMess)
	{
		cout << "Copy constructor called." << endl;
		size_t len = strlen(aMess.pmessage)+1;
		pmessage = new char[len];
		strcpy_s(pmessage, len, aMess.pmessage);
	}
	
	// Move copy constructor
	CMessage(CMessage && aMess)
	{
		cout << "Move copy constructor called" << endl;
		//if(pmessage)
			//delete[] pmessage;
		pmessage = aMess.pmessage;
		aMess.pmessage = nullptr;
	}
	
	// Destructor to free memory allocated by new
	~CMessage()
	{
		cout << "Destructor called." // Just to track what happens
			 << endl;
		delete[] pmessage; // Free memory assigned to pointer
	}
};

int main()
{
	CMessage motto1("Text in motto1 ");
	CMessage motto2("Text in motto2\n");
	CMessage motto3;
	cout << " Executing: motto3 = motto1 + motto2 " << endl;;
	motto3 = motto1 + motto2;
	cout << " Done!! " << endl << endl;
	cout << " Executing: motto3 = motto3 + motto1 + motto2 " << endl;
	motto3 = motto3 + motto1 + motto2;
	cout << " Done!! " << endl << endl;
	cout << "motto3 contains - ";
	motto3.ShowIt();

	system("pause");
	cout << endl;
return 0;
}


Here are the output messages after executing program. (Program runs to completion). I`m currently using VS 2010 in Debug Mode, and added vld.h

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
/*
Visual Leak Detector Version 2.4RC2 installed.
'CMessage.exe': Loaded 'C:\Windows\System32\apphelp.dll', Cannot find or open the PDB file
WARNING: Visual Leak Detector detected memory leaks!

---------- Block 6 at 0x002E4A30: 16 bytes ----------
  Leak Hash: 0xD9ECBCC7, Count: 1, Total 16 bytes
  Call Stack (TID 4500):
    f:\dd\vctools\crt_bld\self_x86\crt\src\newaop.cpp (6): CMessage.exe!operator new[] + 0x9 bytes
    c:\users\alejandro\documents\visual studio 2010\projects\cmessage\cmessage\main.cpp (64): CMessage.exe!CMessage::CMessage + 0x15 bytes
    c:\users\alejandro\documents\visual studio 2010\projects\cmessage\cmessage\main.cpp (26): CMessage.exe!CMessage::operator+ + 0xD bytes
    c:\users\alejandro\documents\visual studio 2010\projects\cmessage\cmessage\main.cpp (105): CMessage.exe!main + 0x1E bytes
    f:\dd\vctools\crt_bld\self_x86\crt\src\crtexe.c (555): CMessage.exe!__tmainCRTStartup + 0x19 bytes
    f:\dd\vctools\crt_bld\self_x86\crt\src\crtexe.c (371): CMessage.exe!mainCRTStartup
    0x74AFEE1C (File and line number not available): kernel32.dll!BaseThreadInitThunk + 0x12 bytes
    0x76F037EB (File and line number not available): ntdll.dll!RtlInitializeExceptionChain + 0xEF bytes
    0x76F037BE (File and line number not available): ntdll.dll!RtlInitializeExceptionChain + 0xC2 bytes
  Data:
    44 65 66 61    75 6C 74 20    4D 65 73 73    61 67 65 00     Default. Message.


Visual Leak Detector detected 3 memory leaks (156 bytes).
Largest number used: 506 bytes.
Total allocations: 558 bytes.
Visual Leak Detector is now exiting.
The program '[4188] CMessage.exe: Native' has exited with code 0 (0x0).
*/


I appreciate very much your help!! thanks!

Alejandro
closed account (2AoiNwbp)
I was wondering whether it has to do with something going on in the stack frame of a constructor, so I copied the code of the constructor to a function, and added an extra bit of code to clean the private member:

1
2
3
4
5
6
7
8
9
10
11
12
13
	// Test function
	void Assign(const char* text = "some text...") // *** NO ERROR !!! *** //
	{ 
		cout << "const char* test function called." << endl;
		if(!text)
			pmessage = NULL;
		else 
		{
			delete[] pmessage;
			pmessage = new char[strlen(text) + 1]; // Allocate space for text
			strcpy_s(pmessage, strlen(text)+1, text); // Copy text to new memory
		}
	}


and added the following line to main()

 
motto3.Assign();


and got no memory leaks!!!

1
2
3
4
5
6
/*
Visual Leak Detector Version 2.4RC2 installed.
'CMessage.exe': Loaded 'C:\Windows\System32\apphelp.dll', Cannot find or open the PDB file
No memory leaks detected.
Visual Leak Detector is now exiting.
The program '[5384] CMessage.exe: Native' has exited with code 0 (0x0). */


could it has to do with something going on in a constructor??? more doubts than certanties... perhaps has to do with the compiler... I don't know
The problem is with operator+. When you create the variable message on line 23 the constructor allocates memory that is pointed to by message.pmessage but on the next line (24) you reassign message.pmessage without first freeing the memory.
closed account (j3Rz8vqX)
Like Peter stated.

Your constructor created a default string before you ever used it.

Then you immediately modify it, without delete the default string.

Line 23: declared an object (which created a new string by default)
Line 24: reinitialized string from object to new string (memory leak).
closed account (2AoiNwbp)
You are absolutely right! It seems I still don't understand dynamic memory very well...

Now, I've added

1
2
if(CMessage.message)
    delete[] CMessage.message;


after

 
CMessage message;


and Memory Leaks have gone forever!
So, it has nothing to do with constructors though... it was only my mistake.
Thank you both Peter and Dput!

Alejandro
You don't have to check if the pointer is null before deleting. Using delete on a null pointer is perfectly safe and will do nothing.

Instead of using delete in operator+ why don't you just pass null to the constructor to prevent the dynamic memory allocation?
 
CMessage message(nullptr);

closed account (2AoiNwbp)
Good! nice and safe. I`ll make it part of my code.

I thought you cannot delete a null pointer, and it would throw an exception, for trying to delete a memory that was not allocated... too much imagination! I better make a simple test before making these kind of assupmtions.

Tks!
I better make a simple test before making these kind of assupmtions.

If you are unsure it's better to look it up somewhere. C++ is not a language suitable to learn by trial and error. Many things have undefined behavior, such as dereferencing a null (or invalid) pointer, division by zero, writing to an array out of bounds, etc. What will happen in such cases are usually not predictable. It might crash the program, or the program might act buggy or it might just "work".
Topic archived. No new replies allowed.