Jump to content
  • Advertisement
Sign in to follow this  
Leo28C

I got violently beaten by a memory allocation bug...

This topic is 3650 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

It's been a while since I was beaten this badly by a single bug. I feel ashamed... Anyway, yes. I don't know where the bug itself is... At first I thought it was some sort of limitation on the PSP (the platform I'm developing for), but then I ported what I have over to Windows/SDL and the exact same bug came up. Here's what I _think_ is the offending method:
void Level::ReadLevel(FILE *file)
{
	
	char *rands[2];
	int which[4];
	
	Width = 24;
	Height = 24;
	
	rands[0] = new char[Width*Height];
	rands[1] = new char[Width*Height];
	
	// Read & decompress the level randomizers
	for (int r = 0; r < 2; r++)
	{
		int bytesRead = 0;
		
		while (bytesRead < Width*Height)
		{
			int count = fgetc(file),
				ch    = fgetc(file);
			
			// HERE: Problematic line!
			memset(rands[r] + bytesRead, ch, count);]
			
			bytesRead += count;
		}
	}
	
	// Tell which randomizer to use for each section of the level
	for (int r = 0; r < 4; r++)
		which[r] = rand() % 2;
	
	// Copy data from the randomizers onto the final level
	for (int y = 0; y < Height; y++)	
	{
		for (int x = 0; x < Width; x++)
		{
			int i = y*Width+x,
				w = i / BLOCK_WIDTH / BLOCK_HEIGHT;
			
			Data = rands[which[w]];
		}
	}
	
	delete rands[0];
	delete rands[1];
}


See the bold line? The program runs flawlessly if I comment that out, but based on the many errors thrown by the debugger (GDB), I think the allocation of rands[] 0 and 1 can be the actual culprit. I can't reproduce the errors right now, but I recall one of them went through a few C-library functions and completely stopped running after a call to malloc(). Others complained about fgetc(), but the file pointer is fine. Someone, please help me! Thanks a lot in advance.

Share this post


Link to post
Share on other sites
Advertisement
After just a cursory glance at the code, you are using the wrong form of delete. When you use new[] you should use delete[].

i.e. delete[] rands[0];
delete[] rands[1];

Share this post


Link to post
Share on other sites
Quote:
Original post by Deventer
After just a cursory glance at the code, you are using the wrong form of delete. When you use new[] you should use delete[].

i.e. delete[] rands[0];
delete[] rands[1];


The fact the O.P has constructed their code in such a way as to require the invocation of delete/delete[] at all is strange.


std::vector<char> rands0(Width*Height);
std::vector<char> rands1(Width*Height);

// or if Width and Height really are compile-time constants,
// there's no need for dynamic allocation at all. Just:

char rands0[Width*Height];
char rands1[Width*Height];

Share this post


Link to post
Share on other sites
For one you never check fgetc()'s return value. The reason why fgetc() returns int instead of char is so that it has enough precision to represent EOF. EOF is returned when an error is encountered or when you reach the end of the file.

Share this post


Link to post
Share on other sites
I used vectors as suggested, I got the exact same error (Exception - Bus error (data).)

I doubt this is the problem, but vectors can be used like this, right?

memset(&rands0[bytesRead], ch, count);


And another thing, what's wrong with delete? It's only used at the end of each level (because that's where Width and Height vary.)

Thanks!

Share this post


Link to post
Share on other sites
fpsgamer: Yes, I realized that. I switched to ifstream (and used ifstream::eof()) but still the same problem.

Share this post


Link to post
Share on other sites
Quote:
Original post by Leo28C
I used vectors as suggested, I got the exact same error (Exception - Bus error (data).)

I doubt this is the problem, but vectors can be used like this, right?

memset(&rands0[bytesRead], ch, count);


Yes that works. It works because the memory that std::vector uses is guaranteed to be contiguous.

However I would encourage you to use something more conventional:

std::fill_n(rands0.begin()+bytesRead, count, ch);

Quote:
Original post by Leo28C
And another thing, what's wrong with delete? It's only used at the end of each level (because that's where Width and Height vary.)
Thanks!


Idiomatic C++ programs avoid manual memory allocation as much as possible. That is why the_edd suggested you use std::vector.

Share this post


Link to post
Share on other sites
Well, it doesn't crash anymore. I still don't know what the cause was, but it seems you guys and your vectors saved me.

I guess I need to learn beyond "C with classes". =)

Thanks all!

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.

Participate in the game development conversation and more when you create an account on GameDev.net!

Sign me up!