|
|
|
This thread is locked; no one can reply to it.
|
| Deleting moved memory crashes |
|
OnlineCop
Member #7,919
October 2006
 |
Original post updated -- 2009/08/22 Thanks for the help so far all. This is the current working version... sorry if only a few of you can compile it. It compiles and runs on my Mac, at least. I've uploaded a more recent source code if anyone wants to check it out. It now supports mouse control. It doesn't take all of the parameters from "config.xml" yet like I want, and there are still several things hard-coded, but those things are being removed as I continue. Please post feedback as you have them; I always welcome comments. ============================================================ I've been trying to get that bubble popping game done for a while. The version I have uploaded to this post isn't interactive yet, but it's not going to be very hard to actually implement a mouse (it may take less than a day to add support for that). I tried compiling it on Windows, since I know the majority of you are on Windows, and I'll probably want my game to be Windows-based more than Mac-based, but for some reason, the src/TimeKeeper.cpp complains about DWORD not being defined when compiling with gcc unless I #include "winalleg.h" right above line 34: 30# ifdef HAVE_SCHED_H
31# include <sched.h>
32# endif
33#else // !defined(_WIN32)
34# include <mmsystem.h>
35static unsigned long int lastTime = 0;
36static LARGE_INTEGER qpcLastTime;
like this: 30# ifdef HAVE_SCHED_H
31# include <sched.h>
32# endif
33#else // !defined(_WIN32)
34# include <winalleg.h>
35# include <mmsystem.h>
36static unsigned long int lastTime = 0;
37static LARGE_INTEGER qpcLastTime;
and even then, I'm still getting a few other compiling errors (but not as many). Anyway, the problem I'm facing is in Arena::~Arena(). Arena contains three std::list<Ball*> lists.
To move the balls from one list to another, I locate the ball in question, then use the std::splice() command to transfer from one list to another. I am STILL able to "see" all the balls in pBalls and pPoppedBalls through the lifetime of the game (I know, because they continue to pop each other and bounce around), so I know the pointers aren't invalidated. However, when I go in and try to delete the memory within Arena::~Arena() on destruction, I am only able to do so on pBalls without causing a SEGFAULT. If I try to do it on pPoppedBalls or pRemovedBalls, I get errors (I had to #if 0...#endif those sections out). This is bad, though, because this is a HUGE memory leak, and I intend on having multiple levels and rounds of gameplay (like if you hit "R", you get to start a new round... This compiles on my Mac without problem using the makefile, but if anyone else can take a look and give me some help, I'd appreciate it. Cookies! P.S. If this looks like a fun project, and someone wants to help with any part of it (sounds, graphics, ... anything ...), I'd love it!
|
|
Tomoso
Member #3,128
January 2003
 |
I'm guessing that's because you only have one version of each Ball* obj created. When you move them from list to list you are only moving a pointer not the actual object. So when you try to delete the same obj more than once, that's where your segfault comes in. Lazy Noob - Blog |
|
OnlineCop
Member #7,919
October 2006
|
Tomoso said: When you move them from list to list you are only moving a pointer not the actual object. That's actually what I was intending to do. Instead of doing an expensive move, I just want to reassign who "owns" the pointer so it can do all the updates. pBalls.size() shows that there are the correct number of balls in it: 50, 25, 10, etc. And pPoppedBalls.size() increases correctly. When I'm calling update(), the balls within pBalls are moving and the balls within pPoppedBalls are growing, shrinking, and being removed correctly, so I at least know that the pointers are going to the actual Ball* objects... So am I actually trying to double-delete the same memory? Also, when I try to delete the memory, I usually do this: 1std::list<Ball*>::iterator itr;
2for (itr = pBalls.begin(); itr != pBalls.end(); ++itr)
3{
4 Ball* ball = *itr;
5 if (ball)
6 {
7 delete ball;
8 ball = NULL;
9 }
10}
11
12for (itr = pPoppedBalls.begin(); itr != pPoppedBalls.end(); ++itr)
13{
14 Ball* ball = *itr;
15 if (ball)
16 {
17 delete ball;
18 ball = NULL;
19 }
20}
So the memory should be freed and then it should be set to NULL, shouldn't it? Then if the next loop (from within pPoppedBalls) gets to the same address, wouldn't it point to NULL instead of to an address? Or has my logic failed me? Should, instead of passing a Ball* around, I be passing the actual Ball object around?
|
|
Tomoso
Member #3,128
January 2003
|
If I'm honest I've never used std::list I prefer vectors, since most of what I want to do doesn't matter if I can move back and forth or not at will. I also don't use iterators, again because I haven't found a use for them where I desperatley required one. I use vectors like so below: 1#include <cstdlib>
2#include <iostream>
3#include <vector>
4
5using namespace std;
6
7int main(int argc, char *argv[])
8{
9 vector<int*> somevector;
10
11 int *a = new int(10);
12 int *b = new int(20);
13 int *c = new int(30);
14 int *d = new int(40);
15
16 int *o = a;
17
18 somevector.push_back(a);
19 somevector.push_back(b);
20 somevector.push_back(c);
21 somevector.push_back(d);
22
23 /* Remove *a from vector */
24 for (int i = 0; i < somevector.size(); i++) {
25 if (somevector.at(i) == o) {
26 somevector.erase(somevector.begin() + i); /* Pointer only removed from vector */
27 }
28 }
29
30 /* Print vector */
31 cout << "B, C, D" << endl;
32 for (int i = 0; i < somevector.size(); i++) {
33 cout << *somevector.at(i) << endl;
34 }
35
36 /* Printe *a */
37 cout << "A: " << *a << endl;
38
39 /* Delete everything within vector */
40 for (int i = 0; i < somevector.size(); i++) {
41 delete somevector.at(i); /* Deletes the actual object but not the pointer within */
42 somevector.at(i) = NULL; /* the vector */
43 }
44 somevector.clear(); /* Clears all pointers within the vector */
45
46 cout << "Vector Size: " << somevector.size() << endl; /* Should be 0 */
47
48 delete a;
49
50 system("PAUSE");
51 return EXIT_SUCCESS;
52}
ps, In my defense I might have made some errors in my haste, not garunteed to work! Lazy Noob - Blog |
|
weapon_S
Member #7,859
October 2006
 |
The fallback case for Timekeeper::sleep seems to be on all the time. |
|
anonymous
Member #8025
November 2006
|
OnlineCop said: So the memory should be freed and then it should be set to NULL, shouldn't it? Then if the next loop (from within pPoppedBalls) gets to the same address, wouldn't it point to NULL instead of to an address? Or has my logic failed me? From that description, yes, your logic has failed you. If you have a pointer to the same object in more than one list, then you are pretty much screwed. (Each list contains a copy of the pointer, setting one to NULL couldn't possible affect others.) You'd need something smarter, like reference-counted pointers (std::tr1::shared_ptr/boost::shared_ptr) which delete the shared object when the last shared_ptr to it goes out of scope. Or you might store all the pointers in one list that you don't mess around with, and finally delete everything in that and not elsewhere. |
|
OnlineCop
Member #7,919
October 2006
|
weapon_S:
EDIT 1: EDIT 2: EDIT 3: It now supports mouse! Score isn't implemented, and there's no way I'm plunging into sound support at this point (I've never been able to do sound, so if there are volunteers, you'll get 100% of the viewable space on any credits screen for your own logo under the "sound" section And if anyone can help me get this so it will compile on Windows (and other platforms), I'd love::) you too.
|
|
anonymous
Member #8025
November 2006
|
OnlineCop said: anonymous, were you able to compile the program, or were you just looking through the code? I wasn't able to compile it (MinGW 4.4.1) and just read the description of your problem. (When I get a makefile, I'm supposed to run make in that directory? - The damned thing didn't find <allegro.h> in the compiler's main include folder with all related errors.) When I tried with the IDE I got the DWORD error with older code, and errors concerning various missing string functions with the newer version (xml code) regardless how the control macros were set (missing str(n)casecmp with _XMLWINDOWS commented and missing _strnicmp etc otherwise). But looking at the source, you seem to have a lot of faith in setting pointers to NULL --- Ok, added the compiler's include path to the makefile. Now I get an error with: //ball.h line 15: BallConstraints::BallConstraints(); //should be: BallConstraints(); Beyond that it is back to the errors with stricmp and such. |
|
OnlineCop
Member #7,919
October 2006
|
anonymous, when I tried on Windows, it said that I was missing winmm.lib, so I uploaded (to the original post) a version where the makefile defaults to Windows and links to winmm, if you have it. Yes, you need to be in the same directory as the makefile to build. From the command prompt, I usually do the following: 1cd prince_popper
2C:\some_dir\prince_popper>make depend
3g++ -MM -MG -I./include ./src/*.cpp > _depend.tmp
4sed -e "s/^\([a-zA-Z0-9_]*\)\.o:/./obj/\1\.o:/" _depend.tmp > ./obj/makefile.dep
5rm -rf _depend.tmp
6
7C:\some_dir\prince_popper>make
8g++ -o "obj/Arena.o" -c -W -Wall -Wno-unused -pg -g3 -ggdb3 -O0 -DDEBUG -I./include ./src/Arena.cpp
9g++ -o "obj/Ball.o" -c -W -Wall -Wno-unused -pg -g3 -ggdb3 -O0 -DDEBUG -I./include ./src/Ball.cpp
10g++ -o "obj/BallKeyframe.o" -c -W -Wall -Wno-unused -pg -g3 -ggdb3 -O0 -DDEBUG -I./include ./src/BallKeyframe.cpp
11g++ -o "obj/Game.o" -c -W -Wall -Wno-unused -pg -g3 -ggdb3 -O0 -DDEBUG -I./include ./src/Game.cpp
12g++ -o "obj/Main.o" -c -W -Wall -Wno-unused -pg -g3 -ggdb3 -O0 -DDEBUG -I./include ./src/Main.cpp
13g++ -o "obj/TimeKeeper.o" -c -W -Wall -Wno-unused -pg -g3 -ggdb3 -O0 -DDEBUG -I./include ./src/TimeKeeper.cpp
14g++ -o "obj/Timer.o" -c -W -Wall -Wno-unused -pg -g3 -ggdb3 -O0 -DDEBUG -I./include ./src/Timer.cpp
15g++ -o "obj/xmlParser.o" -c -W -Wall -Wno-unused -pg -g3 -ggdb3 -O0 -DDEBUG -I./include ./src/xmlParser.cpp
16g++ -o "./bin/test" -W -Wall -g3 -ggdb3 -O0 -pg ./obj/Arena.o ./obj/Ball.o ./obj/BallKeyframe.o ./obj/Game.o ./obj/Main.o ./obj/TimeKeeper.o ./obj/Timer.o ./obj/xmlParser.o -lalld -lwinmm
17
18C:\some_dir\prince_popper>
You'll need sed for the "make depend" part, liballd.lib and libwinmm.lib (or liballd.a and libwinmm.a, depending on how your system names library files) in your library path, and whatever dependencies stricmp was complaining about earlier. Thanks about noting the problem with Ball.h's "BallConstraints::BallConstraints();"... why the compiler didn't yell at me, I don't know. Thanks. This now compiles and runs fine on my WinXP with no modifications. Can anyone else test and try this version? Again, attachment is on the OP. Feedback welcome and appreciated!
|
|
TeamTerradactyl
Member #7,733
September 2006
 |
Nice.
|
|
OnlineCop
Member #7,919
October 2006
|
Updated original attachment again. I really would like to learn Events one of these days. But for everyone (anyone?) interested in this project or thread, here are a few changes:
Comments on the current version always welcome, and if this DOES compile for others out there, let me know. Controls: Left mouse click (you can only "drop" a ball onto a playing field once per round... the whole point of the game: maximize your score). key[KEY_R]: Reset field, play another round/level You can also use some command-line arguments to change a few of the settings. Like the dimensions of the arena (a long narrow strip would make it very easy to win, hint-hint). You can find those by reading through the source code yourself.
|
|
|
), but take a look (for those who can).
E.g in destructors - this may only help you catch errors where you access an already deleted object (it's members will be NULL), as long as that memory doesn't happen to be occupied by some new object.