C++

Forum

 
Heap Corruption on delete

chuvak (67)
I am manually dealing with memory management and pointers, and I am running into an issue. I will post the whole code below, but I think it is an issue with my assignment operator overload.

When I leave the for loop out of int main() from lines 200 to 209, it works as it should. But once I uncomment the for loop, my call stack tells me my destructor is bad. Im a bit new and confused, so any help would be much appreciated.

I think the issue is memory mismanagement. When Contact tmp gets created, it is the first to be deleted... but once it is deleted, theContact.m_pNumber is no longer pointing to anything, (hence the mismanagement). How can I fix this?

(expected output at the bottom)

Below code:
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
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
 
//header <--------------------------------------------//--------------------------------|
#include <iostream>
#include <cstring>
#include <sstream>
using namespace std;

#ifndef SICT_CONTACT_H
#define SICT_CONTACT_H

namespace sict {

	class Contact {
		char name[20];
		long long* m_pNumber;
		short amtNumbers;
	public:
		Contact();
		Contact(const char* tempName, const long long* tempNumber, const short amount);
		~Contact();
		bool isEmpty() const;
		void display() const;
		void setEmpty();
		bool isValidPhonenumber(const long long phone);
		//at home
		Contact(const Contact &other);
		Contact& operator=(const Contact& other);
		Contact& operator+=(long long phone);
	};
}
#endif // !SICT_CONTACT_H

//implementation <--------------------------------------------//--------------------------------|
namespace sict {
	Contact::Contact() {
		setEmpty();
	}

	Contact::Contact(const char* tempName, const long long* tempNumber, const short amount) {
		setEmpty();
			if ('\0' != tempName && nullptr != tempName) {
				for (int i = 0; i < strlen(tempName) + 1; i++) {
					name[i] = tempName[i];
				}
			}
			if (nullptr != tempNumber) {
				for (int i = 0; i < amount; i++) {
					if (isValidPhonenumber(tempNumber[i])) {
						amtNumbers++;

					}
				}
				m_pNumber = new long long[amtNumbers];
				int count = 0;
				for (int i = 0; i < amount; i++) {
					if (isValidPhonenumber(tempNumber[i])) {
						m_pNumber[count] = tempNumber[i];
						count++;
					}
				}
			}
	}

	Contact::Contact(const Contact &other)
	{
                strcpy(name, other.name); //<---------------- added!
		this->amtNumbers = other.amtNumbers;
		this->m_pNumber = new long long[this->amtNumbers];
		for (int i = 0; i < this->amtNumbers; ++i) {
			this->m_pNumber[i] = other.m_pNumber[i];
		}
		
	}

	Contact& Contact::operator=(const Contact& other)
	{
		if (this != &other) {
			strcpy(this->name, other.name);
			delete[] this->m_pNumber;
			this->amtNumbers = other.amtNumbers;
			this->m_pNumber = new long long[amtNumbers];
			for (int i = 0; i < this->amtNumbers; ++i) {
				this->m_pNumber[i] = other.m_pNumber[i];
			}
			return *this;
		}
		else {
			return *this;
		}
	}

	Contact& Contact::operator+=(long long phone)
	{
		if (isValidPhonenumber(phone) && !isEmpty()) {
			long long* tmp = new long long[this->amtNumbers + 1];
			for (int i = 0; i < this->amtNumbers; i++) {
				tmp[i] = this->m_pNumber[i];
			}
			tmp[amtNumbers] = phone;
			delete[] this->m_pNumber;
			this->m_pNumber = tmp;
			this->amtNumbers++;
			return *this;
		}
		else {
			return *this;
		}
	}

	Contact::~Contact() {
		delete[] m_pNumber;
	}

	bool Contact::isEmpty() const {
		if ('\0' == name[0]) {
			return true;
		}
		else {
			return false;
		}
	}

	void Contact::setEmpty() {
		name[0] = '\0';
		m_pNumber = nullptr;
		amtNumbers = 0;
	}

	bool Contact::isValidPhonenumber(const long long phone)
	{
		short first, second, third, fourth;
		if (0 > phone || 1000000000000 <= phone || 10000000000 > phone) {
			return false;
		}
		if (phone >= 100000000000) {
			first = phone / 100000000000;
			second = (phone / 10000000000) % 10;
			third = (phone / 1000000000) % 10;
			fourth = (phone / 1000000) % 10;
			if (first == 0 || second == 0 || third == 0 || fourth == 0) {
				return false;
			}
		}
		if (phone >= 10000000000) {
			first = phone / 10000000000;
			third = (phone / 100000000) % 10;
			fourth = (phone / 100000) % 10;
			if (first == 0 || third == 0 || fourth == 0) {
				return false;
			}
		}
		return true;
	}

	void Contact::display() const {
		if (isEmpty()) {
			cout << "Empty contact!" << endl;
		}
		else {
			int sizeName = strlen(name);
			for (int i = 0; i < sizeName; i++) {
				cout << name[i];
			}
			cout << endl;
			for (int i = 0; i < amtNumbers; i++) {
				if (m_pNumber[i] >= 100000000000) {
					short first = m_pNumber[i] / 100000000000;
					short second = (m_pNumber[i] / 10000000000) % 10;
					short third = (m_pNumber[i] / 1000000000) % 10;
					cout << "(+" << first << second
						<< ") " << third << (m_pNumber[i] / 100000000) % 10 << (m_pNumber[i] / 10000000) % 10
						<< " " << (m_pNumber[i] / 1000000) % 10 << (m_pNumber[i] / 100000) % 10 << (m_pNumber[i] / 10000) % 10
						<< "-" << (m_pNumber[i] / 1000) % 10 << (m_pNumber[i] / 100) % 10 << (m_pNumber[i] / 10) % 10 << (m_pNumber[i] / 1) % 10
						<< endl;
				}
				else if (m_pNumber[i] >= 10000000000) {
					short first = m_pNumber[i] / 10000000000;
					short second = (m_pNumber[i] / 1000000000) % 10;
					cout << "(+" << first
						<< ") " << second << (m_pNumber[i] / 100000000) % 10 << (m_pNumber[i] / 10000000) % 10
						<< " " << (m_pNumber[i] / 1000000) % 10 << (m_pNumber[i] / 100000) % 10 << (m_pNumber[i] / 10000) % 10
						<< "-" << (m_pNumber[i] / 1000) % 10 << (m_pNumber[i] / 100) % 10 << (m_pNumber[i] / 10) % 10 << (m_pNumber[i] / 1) % 10
						<< endl;
				}
			}
		}
	}
}

//main <--------------------------------------------//--------------------------------|
int main() {
    sict::Contact theContact("John Doe", nullptr, 0); // sict:: intentional
    theContact.display();
    theContact += 14161234567LL;
    theContact += 14162345678LL;
    theContact += 14163456789LL;
    theContact += 114164567890LL;
    theContact.display();

    cout << endl << "Testing! Please wait:" << endl;
	
    for (int i = 1; i <= 500000; i++)
    {
        sict::Contact temp = theContact;
        theContact = temp;
        theContact = theContact;
        if (!(i % 10000))
            cout << ".";
        if (!(i % 500000))
            cout << endl;
    }
	
    cout << endl;
    theContact.display();

	theContact = sict::Contact("", nullptr, 0);
	theContact += 14161230002LL;
	theContact.display();
	
    return 0;
}


Expected output:
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
 
John Doe
John Doe
(+1) 416 123-4567
(+1) 416 234-5678
(+1) 416 345-6789
(+11) 416 456-7890

Testing! Please wait:
..................................................
..................................................
..................................................
..................................................
..................................................
..................................................
..................................................
..................................................
..................................................
..................................................

John Doe
(+1) 416 123-4567
(+1) 416 234-5678
(+1) 416 345-6789
(+11) 416 456-7890
Empty contact!
Last edited on
Ihatov (75)
The problem is probably that inside your copy constructor (line 63) you've never copied the name.
Last edited on
jonnin (11429)
that error almost always means you accessed memory outside your bounds in the program, and when you went to delete it, the delete was confused due to the bounds violation.
TheIdeasMan (6817)
I am manually dealing with memory management and pointers, ....


Why? Why not use the STL, it was invented to avoid the problems that come from doing that.

I find it interesting that you have a phone number as a long long. They have spaces, hyphens, +, () , so why not make it a std::string?
Kourosh23 (178)
Ok I didn't read your code, but based on your question and my past experience with memory corruption error I think the problem is that somewhere in your code you are using delete when nothing has been previously made or maybe calling it more than it is necessary.

Note: deleting something that does not exist will cause a memory corruption.

I faced this issue with my copy constructor, overload assignment operator, and remove functions (for removing elements from an array, stack, or queue).

Hope this helps you figure out the source of error.
TheIdeasMan (6817)
Kourosh23's comment bring up another thing: Avoid using new and delete, unless you are writing a library.

More reasons to use the STL.
chuvak (67)
So the issue was simply adding strcpy(name, other.name)' into line 65 in the copy constructor... odd. Everything works now. Thanks everybody.

As for long long, it is a specification of my professor. I shouldnt change main().

Regarding STL, I havent learned it yet, so I can use it unfortunately.
Last edited on
Topic archived. No new replies allowed.