I have an AVL tree with string keys, and my own String class. In order to fix another issue, I had to add a copy constructor to String. However, valgrind reports an error with it. Here's the constructor:
String::String(const String& s){
try {
mStr = new char[1];
}
catch (const bad_alloc& e){
printf("ERROR: No memory\n");
exit(0);
}
mStr[0]='\0';
mCopy(s);
}
void String::mCopy(const String& s){
// delete[] mStr;
size_t n = s.Length();
try{
mStr = new char[n + 1];
}
catch (const bad_alloc& e){
printf("ERROR: No memory\n");
exit(0);
}
strcpy(mStr, s.mStr);
mStr[n + 1] = '\0';
}
And here's a part of Valgrind output after adding a string key to my AVL tree:
==7005== Invalid write of size 1
==7005== at 0x4013A1: String::mCopy(String const&) (in /home/ezrafell/Desktop/DA2final/DA3.o)
==7005== by 0x401213: String::String(String const&) (in /home/ezrafell/Desktop/DA2final/DA3.o)
==7005== by 0x40182D: main (in /home/ezrafell/Desktop/DA2final/DA3.o)
==7005== Address 0x5ad450d is 0 bytes after a block of size 61 alloc'd
==7005== at 0x4C2E80F: operator new[](unsigned long) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==7005== by 0x40136B: String::mCopy(String const&) (in /home/ezrafell/Desktop/DA2final/DA3.o)
==7005== by 0x401213: String::String(String const&) (in /home/ezrafell/Desktop/DA2final/DA3.o)
==7005== by 0x40182D: main (in /home/ezrafell/Desktop/DA2final/DA3.o)
Other errors reported by Valgrind also trace back to operator new[]
in String::String(String const&)
. But what's wrong with it? And how do I rewrite the constructor to avoid this error?
The error is produced by this line:
mStr[n + 1] = '\0';
n+1
is one past the allocated size. You need to use n
instead, because the body of the string is located at indexes 0
through n-1
, inclusive, so the index n
is where null terminator goes:
mStr[n] = '\0';
Note Removing this line will be correct, too, because strcpy
null-terminates its result. Placing null terminator manually is required when you use functions that do not do it for you, such as memcpy
.