next »

[Alex]

Hey guys, hopefully someone can give me a hand with this. I'm trying to write a basic tile engine and I thought I'd be fancy and use some classes and all kinds of crap. Long story short, it all works right up until I try to free up some memory from my map class.

Here's what I use to create the array of Tiles (another class):

Code:

this->tiles = (Tile **)malloc(sizeof(Tile *)*x);
for (int i = 0; i <= x; i++) {
    this->tiles[i] = (Tile *)malloc(sizeof(Tile)*y);
}

And nothing I use to free it seems to work without giving some indecipherable errors about heaps and death and destruction being rained down on the unsuspecting bits of my RAM.

I've tried these so far:

Code:

delete [] *tiles;
delete [] tiles;

for (int i = 0; i <= x; i++) {
     free(this->tiles[i]);
}
free(this->tiles);

And various combinations of the two.

If anyone could give me some pointers (no pun intended) to help me in the right direction that'd be awesome. If you need more source code I'd be happy to provide.

[Alex May]

Never mix new/delete and malloc/free.

If you allocate with malloc, free it with free. If you allocate with new, free it with delete.

If you are using classes and C++, tend to use new and delete. If you are using structs and C, tend to use malloc and free.

your <= in the for loop is wrong too (you're allocating x tiles but assigning to x + 1)

e.g.

Code:

// e.g. class Map { Tile*** tiles };
tiles = new Tile**[width];
for (int i = 0; i < width; i++) {
  tiles[i] = new Tile*[height];
  for (int j = 0; j < height; j++) {
    tiles[i][j] = new Tile();
  }
}

Code:

// e.g. class Map { Tile*** tiles };
for (int i = 0; i < width; i++) {
  for (int j = 0; j < height; j++) {
    delete tiles[i][j];
  }
  delete[] tiles[i];
}
delete[] tiles;

think so

[henzenmann]

And nothing I use to free it seems to work without giving some indecipherable errors about heaps and death and destruction being rained down on the unsuspecting bits of my RAM.

I've tried these so far:

Code:

delete [] *tiles;
delete [] tiles;

for (int i = 0; i <= x; i++) {
     free(this->tiles[i]);
}
free(this->tiles);

And various combinations of the two.

If anyone could give me some pointers (no pun intended) to help me in the right direction that'd be awesome. If you need more source code I'd be happy to provide.

Don't mix malloc/free and new/delete. Without the delete statements that code actually looks correct t me. I think you need to look at the context this code runs in (how/when/in what order is it called, how is 'x' set...) to find the error... finding out what those "indecipherable errors" mean would also be a good start

[Alex]

Ah that was stupid of me! Adding (x+1) to malloc worked fine. Thanks a bunch!

I'll have a crack at converting it to new/delete since I am using classes, though it's a bit unfamiliar to me.

EDIT: Done. Your allocation was perfect, but the delete that finally worked for me was:

Code:

for (int i = 0; i < maxX; i++) {
    for (int j = 0; j < maxY; j++) {
        delete tiles[i][j];
    }
    delete [] *tiles[i];
}
delete [] **tiles;

It also took me a while to figure out that to use the tile object I had to do

Code:

(*tiles[i][j]).function()

It didn't work without the extra brackets.

[Alex May]

To call a member function of an object instance to which you have a pointer, you use the -> operator, although (*pointer).function() does work of course. Use pointer->function() instead.

Your deletes look wrong to me. *tiles[ i ] is of type Tile*, not Tile**, and equal to tiles[ i ][0], which you just deleted. What happens when you use the code I gave you?

Read all of this, particularly this.

[mirosurabu]

Hi Alex,

Is there any reason not to use STL vectors?

Working with C-style arrays can be very troubling.

[Ivan]

I second that suggestion.

[Alex]

To call a member function of an object instance to which you have a pointer, you use the -> operator, although (*pointer).function() does work of course. Use pointer->function() instead.

Thanks, I'm not really all that familiar with classes in C++.

Your deletes look wrong to me. *tiles[ i ] is of type Tile*, not Tile**, and equal to tiles[ i ][0], which you just deleted. What happens when you use the code I gave you?

Read all of this, particularly this.

Your code gives:

Code:

Heap block at 00BE1FA8 modified at 00BE1FD4 past requested size of 24

Thanks for the link, it was quite helpful. I understand why the code I posted above is wrong now, but I'm still at a loss as to the correct code.

What I'm doing at the moment will delete all the individual Tiles (three times each >.>), but leave the pointers for each row and the pointer to those pointers. Is that correct? Your code should be deleting both of those, and I understand why it should be, but it isn't for some reason.

I'm compiling this using Visual Studio Express 2008, if that helps.

Hi Alex,

Is there any reason not to use STL vectors?

Working with C-style arrays can be very troubling.

To be honest, I only really know C, but since I'm using a C++ compiler and know Java OOP fairly well I thought I'd chuck a couple of objects. I've never used vectors before, but it does look quite similar to a Java List. I might give it a shot if I can't get this working.

EDIT: Ok I've redone it with vectors. I had one of my compiler options configured wrong, which for some reason only chose to explode the linker when using vectors. But anyway, half an hour and half a glass of caffeine later it works! I'm still interested in the solution to the delete problem above if anyone can figure it out.

[Mikademus]

You know, since you're set on making an engine that you will use several times, please do take the time to read up on and learn proper C++. You can't program C++ as you would C and expect good results any more than you can program C++ as you would Java. Especially study the STL. For instance, look through Herb Sutter's "Effective C++" and keep a reference copy of "The C++ Programming Language, 3rd Ed" around. Just sayin'.

[skyy]

I agree with Mikademus. Learning basic STL usage when doing C++ is a must. Well of course you can skip it, I'm not saying that but everyone who does C++ should also atleast KNOW the capabilities and functionality of C++ Standard Template Library. Also, I suggest that when you get good enough with C++, reading and learning Boost is also a great asset to your arms when it comes to programming in C++.

Boost contains all sorts of sexyness! Boost ftw! 

Also, to agree with few other posters above, never mix malloc/free and new/delete
Malloc/free is generally used with C and new/delete is generally used with C++. Plus new/delete are alot easier to use when learning the language since you don't have to do any fancy "sizeof(type)" and so on.

[Mikademus]

I'd like to skyy's post that when programming C++ correctly you'll actually very seldom use dynamic memory allocation (new and delete). When studying C++ make a point of reading up on references and if you ever need to dynamically allocate an object with new, always store that pointer in a smart pointer or at least an auto_ptr. Do NOT use your Java habits.

[mcc]

Even if you don't know the c++ / stl classes, it will be easier to learn them than it will be to internalize how to do C memory management properly...

[Core Xii]

when programming C++ correctly you'll actually very seldom use dynamic memory allocation (new and delete). When studying C++ make a point of reading up on references and if you ever need to dynamically allocate an object with new, always store that pointer in a smart pointer or at least an auto_ptr.

What? I don't agree with any of this. Most if not all applications allocate objects dynamically - If they don't, you're doing it wrong. You're not supposed to know everything at compile time, you're supposed to load object definitions from scripts to have them easily editable without recompilation.

Also, what's with the smart pointers? If you have any idea what you're doing you can do a much better job manually deallocating your stuff.

[Ivan]

Yeah i don't know what that was all about. Dynamic memory allocation is the center of any real application. The point is that STL is there for a reason and unless you're confident in your ability to make more optimized containers, you should use STL's.

[Alex]

I'm actually just making this engine as a learning excersize to familiarise myself with SDL. That happened to mean I'm using C++ instead of C, which I'm more used to, and that was what made me decide to use some OOP. So yeah, I kind of quickly got out of my depth, but I've always found I learn better by doing rather than reading so I'll keep at it. I've got a C++ book around here somewhere so I'll try and find it and read up on STL since it seems you guys think quite highly of it

So yeah that's my excuse for being hopeless at this! But thanks for all the suggestions and help.

EDIT: I found my C++ book, it's from 1994, is that going to be a problem? It's called 'C++ with Object-Oriented Programming' by Paul S. Wang if anybody's heard of it.

[BorisTheBrave]

I agree with Mikademus, nowadays, raw allocation and deallocation are not good, you should almost always allocate dynamically using smart pointers or containers. (as a point of order, using smart pointers still means dynamic allocation using new).

It makes writing exception safe code so much easer (i.e. possible), and it frequently saves effort of having to manually track the appropriate time for allocation/deallocation. I'm not sure what you mean you can do a much better job than smart pointers - they are reference counted not GCed, so assuming you haven't screwed up, they will de-allocate at exactly the correct opportunity?

No one's written it, so I'll just spell out using std::vector for those who aren't familiar, but interested.

Code:

using namespace std;
vector<vector<Tile> > tiles(width);
for(int i=0;i<width;++i)
  tiles[i].resize(height);

Access a tile with tiles[ x ][ y ]. No deletion code necessary. I hope this spells out why it's a better alternative that mucking around in arrays.

[Core Xii]

I'm not sure what you mean you can do a much better job than smart pointers - they are reference counted not GCed, so assuming you haven't screwed up, they will de-allocate at exactly the correct opportunity?

And here is the problem. Who knows best what the exact, correct opportunity is, you or the computer? You're essentially delegating this task to your program at the cost of resources. When you know where to deallocate your stuff, no memory or CPU is wasted by the program trying to figure it out.

[BorisTheBrave]

By correct opportunity, I meant as soon as you have no further references to it. As you cannot de-allocate before without invalidating some references, and any later is a waste of resources. Quite frequently, I will not know when this is, because we are only reaching for pointer/smartpointer usage after it's been determined that static allocation / std containers aren't going to cut it, so it must be a pretty complicated setup. The only examples I can think of where the overhead is actually redundant are when writing containers of some variety in the first place, which is fair enough. Certainly, it's impossible to have nontrivial allocations managed without some memory or CPU hit, as you seem to suggest.

Essentially smart pointers save you work, at a cost which is 80% of the time too small to be relevant. You should write your program first, and worry about undue memory consumption second. They have other benefits, too, like exception safety, and easier decoupling between modules.

[increpare]

Code:

using namespace std;
vector<vector<Tile> > tiles(width);
for(int i=0;i<width;++i)
  tiles[i].resize(height);

Access a tile with tiles[ x ][ y ]. No deletion code necessary. I hope this spells out why it's a better alternative that mucking around in arrays.

Easier still with boost multiarray

Code:

tiles.resize(extents[width][height]);

heh

[raigan]

FWIW, there have been recent threads on the gdalgorithms list, and a lot of people still recommend using _static_ allocation for games, so all the "dynamic everything" talk should be taken with a grain of salt.

The thing is, you're always going to have some upper limit on the number of objects that can be handled at once -- your collision system, graphics system, etc. will be a bottleneck so you can't realistically handle an unlimited number of objects. Once you have a finite upper limit on the number of objects, you can simply statically pre-allocate MAX_NUM of each type of object and use those.

Obviously there are various ways of doing this (i.e use a stack-type allocator for all per-level data so that when loading a new level you can easily wipe all the existing level data), but the basic idea of "really, nothing should be unknown at runtime" is often sound. Although it's not "free", you have to design for this sort of use (i.e your particle systems, bullet-bill-launchers, etc should have a max number of "children" objects that can be spawned, and manage/re-use them, rather than simply spawning them and forgetting about them).

Of course using dynamic allocation is probably great on PC, if you're used to that sort of thing, etc. -- I just wanted to point out that it's perfectly valid to NOT use new() at all for in-game allocations, and that AFAIK many console games especially avoid dynamic allocation.