Is there any reason that this code should cause this error as soon as it hits the delete[] call?
Error:
Windows has triggered a breakpoint in Nocturne.exe.
This may be due to a corruption of the heap, and indicates a bug in Nocturne.exe or any of the DLLs it has loaded.
The output window may have more diagnostic information
Code:
for(int i = 0;i<fileStruct.tileCount;i++)
{
UINT size = 0;
// Read the length of the string from the file
reader.read((char*)&size, sizeof(UINT));
char* buff = new char[size + 1];
// Read the string from the file
reader.read(buff, stringSize);
// NULL-terminate the string
buff[stringSize] = '\0';
fileStruct.mapIDs = buff;
delete[] buff;
buff = NULL;
}
To elaborate a little bit, in many cases, crashing on delete [] is not a problem with that line itself, but could be a problem with writing beyond the boundaries of the array somewhere earlier in the program. It is only when you try to delete the array that it realizes that there has been an improper write at some previous point in time. As SiCrane mentioned, your problem in this case is that you write to the [stringSize] index of your array, but your array actually only contains elements [0] through [size] (size + 1 total), and if stringSize is greater than size, you would have written beyond the boundaries of the memory that you allocated.
A common addition to many standard library functions to make them safer in recent years has been to add a buffer size or max elements parameter to any read() function. So you could call something like "reader.read(buff, size + 1, stringSize);", and within the read() function, you could make sure to not write more than size + 1 characters to the buffer, and thus ensure that you don't overflow the buffer.
I personally prefer to use std::vector, using .resize() to initialize the buffer, and passing a reference in to the read() function. Then the read() function can use .size() to know for sure how big it is (or could even resize it, if such functionality was appropriate). This tends to be cleaner and safer than the standard pointer/buffer operations of C-style programming, in my opinion at least.
[edit]I should point out that some of what I discussed depends on being able to modify the reader class. If that class is someone else's, then it might be difficult (or impossible, in the case of an already compiled DLL) to modify it.[/edit]
"We should have a great fewer disputes in the world if words were taken for what they are, the signs of our ideas only, and not for things themselves." - John Locke
Quote:Original post by _Sigma Is there any reason that this code should cause this error as soon as it hits the delete[] call?
A common cause of te same error I've seen is a double deletion (deleting the same pointer twice). It looks like the code you poseted is not really the code you've got that causes the problem. You're assigning the pointer to a vector of some sort (mapIDs) and then deleting the pointer. Is there some code you've elided between the assignment and the delete? Does the assignment to the vector clone the memory pointer to by the pointer?
Even if you've got an array walker or a buffer overflow, it looks suspiciously like you've left some key code out of your post.
Stephen M. Webb
Professional Free Software Developer
Umm, "fileStruct.mapIDs" *is* a std::string, yes? If it's a char*, then it ends up pointing at the thing you then delete[].
If it is a std::string, you should be aware that you don't need a temporary buffer. One of many other ways:
for(int i = 0;i<fileStruct.tileCount;i++) { UINT size = 0; // Read the length of the string from the file reader.read(reinterpret_cast<UINT>&size, sizeof(UINT)); fileStruct.mapIDs.clear(); for (int i = 0; i < size; ++i) { char c; reader.get(c); fileStruct.mapIDs.append(c); }}
The problem is that you read the file size into "size" and then used different variable "stringSize" to read the data. Not sure what that variable is but it doesnt contain the size you just read in.
So "stringSize" is probably unitialized and your overruning your buffer, which is causing delete to give you a nice warning.
Ultimately, this is a long way of saying "Well the obvious answer is if stringSize is greater than size".
Quote:Original post by Zahlman Umm, "fileStruct.mapIDs" *is* a std::string, yes? If it's a char*, then it ends up pointing at the thing you then delete[].
Zahlman is right, you create a new string and have the mapId = to the string buffer then delete it. In C++ strings and arrays aren't copied into a new memory buffer when you see an equal sign, the pointer to the memory of the string or array is copied.
I'm so embarrassed. I was using stringsize instead of size...bah. So it runs w/o crashing now, but is what I did, assigning the buffer to mapID wrong? It *is* a std::string.
Quote: Zahlman is right, you create a new string and have the mapId = to the string buffer then delete it. In C++ strings and arrays aren't copied into a new memory buffer when you see an equal sign, the pointer to the memory of the string or array is copied.
This would imply that i'm doing something wrong. What should I do to correct this problem?
Quote:Original post by SiCrane If it is a std::string then what you have is fine.
Right. std::string(char*) will copy the data into the string's internal buffer, and then the temporary allocation can safely be deleted. But you might still want to not use a temporary allocation - after all, you've just seen first-hand how error-prone it can be ;)