Jump to content
  • Advertisement
Sign in to follow this  
DangerDave

Cleaning up null terminated strings

This topic is 5035 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

After much searching I've found the source of a memory leak in my program. I have global functions for writing debug data to a null terminated string:
static void AppendDebugText(const char* text){

	if (DEBUGGING){
		char* tmp = new char[strlen(text) + strlen(g_DebugText) + 1];
		
		for (unsigned int i = 0; i < strlen(g_DebugText); i++) tmp = g_DebugText;

		for (unsigned int j = 0; j < strlen(text) + 1; j++) tmp[i++] = text[j];

		delete [] g_DebugText;
		g_DebugText = new char[strlen(tmp) + 1];
		strcpy(g_DebugText, tmp);
		delete [] tmp;
	}
}

When I set DEBUGGING false there is no memory leak. None of strings passed are declared with the new operator, all on the stack. There are a couple of other functions similar to this one, all following the same layout. What am I doing wrong? Any help much appreciated.

Share this post


Link to post
Share on other sites
Advertisement
You never delete[] the last allocated instance of g_DebugText. The solution? std::string:
static std::string g_DebugText;

static void AppendDebugString(const std::string& text)
{
if (DEBUGGING)
{
g_DebugText += text;
}
}


Enigma

Share this post


Link to post
Share on other sites
Ah, I do delete the last instance elsewhere. The problem is there is a memory leak whilst the program is running. ctrl-alt-del'ing and checking how much memory the process is using shows the memory usage constantly increasing whilst the program is running (AppendDebugText as well as some others are called every time step).

I'd rather not use a string, it would mean a lot of hassle in the rest of the code and I would be no the wiser why this problem is happening. But, then, if I cant find a solution I'll have to.

Share this post


Link to post
Share on other sites
Well, if you delete[] the last instance then there is no memory leak in that code snippet, although it's not exception safe and in fact not valid C++ (The C++ defines the scope of a variable declared within a for loop statement to be the same as the scope of the for loop. You must be using either Visual C++ 6 or a later version of Visual C++ without enforcing standard for loop scoping).

Task manager is not a reliable way to detect memory leaks and your 'leak' may not actually exist. There is a set of functions in Visual C++ to monitor memory usage (crtdbg?) - try using that instead.

That said, std::string really is what you should be using for textual data in C++ and converting existing code to use it should not be too difficult.

Enigma

Share this post


Link to post
Share on other sites
Way too much work going on.

static void AppendDebugText(const char* text){
if (DEBUGGING){
char* tmp = new char[strlen(text) + strlen(g_DebugText) + 1];
strcpy(tmp, g_DebugText);
strcat(tmp, text);
delete [] g_DebugText;
g_DebugText = tmp;
}
}

Share this post


Link to post
Share on other sites
Quote:
Original post by Oluseyi
Way too much work going on.
*** Source Snippet Removed ***
Exactly what I was going to say.
It was pointless to allocate memory the same size as tmp, then copy the data into it from tmp, then delete tmp. Why not just do g_DebugText = tmp; and forget allocating g_DebugText and deleting tmp.

btw there is no leak, but you might think there is because tmp gets bigger by strlen(text)+1 each call to AppendDebugText.

It's gonna get slower and slower too this way as after a while you're going to be copying a huge amount of memory each time. You should preallocate g_DebugText to say 64K beforehand rather than constantly creating a bigger buffer each time. That will be much faster since you can merely append and you're done. Check that you don't exceed the buffer though.

Although, why not write it to file instead?
What use is keeping all of the debug info around in memory anyway?

Share this post


Link to post
Share on other sites
Simple ...


freopen("errors.txt", "w", stderr);
...
fprintf(stderr, "Heres a number: %d\n", 10);
...
fclose(stderr);


Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

We are the game development community.

Whether you are an indie, hobbyist, AAA developer, or just trying to learn, GameDev.net is the place for you to learn, share, and connect with the games industry. Learn more About Us or sign up!

Sign me up!