Segmentation fault while cleaning up program

Alex Alifimoff Source

I'm very new to C, and I've been going through Zed Shaw's "Learn C the Hard Way". I have experience in other languages, but mostly languages that handle their own memory management and so I'm new to some of this. I built the game described in this lesson. and extended it slightly so I could define the game mechanics in one class and build the maps in another class. I am trying to build functions to handle the memory management and free all of the heap memory allocated by the program, but my program causes a segmentation fault while cleaning up.

I have defined a room object, based on an object super class, like this:

struct Room {
    Object proto;

    Monster *bad_guy;

    struct Room *north;
    struct Room *south;
    struct Room *east;
    struct Room *west;
};

typedef struct Room Room;

Object RoomProto;

I have this method called when the map is destroyed. The program causes a segmentation fault, according to valgrind, on the line destroying room->north. My thought behind this was that I'd check to see if the room existed and then destroy it if it did, that way I could go through all of the map and delete each room but not try to free any NULL pointers.

void Room_destroy(void *self)
{
    Room *room = self;


    if(room){
        if(room->north){
            room->north->_(destroy)(room->north);
        }
        if(room->south){
            room->south->_(destroy)(room->south);
        }
        if(room->west){
            room->west->_(destroy)(room->west);
        }
        if(room->east){
            room->east->_(destroy)(room->east);
        }
        if(room->bad_guy){
            room->bad_guy->_(destroy)(room->bad_guy);
        }
        room->_(destroy)(room);
    }
}

I have tried to solve the problem myself in a couple of ways, like first assigning pointers to null, like: Room *n = NULL; n = room->north; and then checked n instead of just room->north. I know this is a common error and so I feel like I may just be missing some critical point about pointers, no pun intended.

cpointerssegmentation-fault

Answers

answered 4 years ago nvoigt #1

What does this line even do? Is that standard C?

if(room->north){
        room->north->_(destroy)(room->north);
    }

This is the standard way of memory management:

if(pointer != NULL)
{
   free(pointer);
   pointer = NULL;
}

Replace pointer with any pointer you have, for example room->north and you are good. Don't forget to set all your pointers to either a piece of allocated memory or NULL from the start.

answered 4 years ago Arpit #2

OK, So it seems that the link you provided is implementing OOP in C. That's bad. If you are trying to learn C then it will be better to try to understand the basics first. This will only confuse you.

Now comes to your code. I tried to understand and here seems to be the problem. You are doing

if(room->north){
     room->north->_(destroy)(room->north);
}
...

You are calling the destroy function of the same object which you are passing as a parameter. So you are in the memory which you want to free. If you want to do like this then make it

if(room->north){
     room->_(destroy)(room->north);
}
...

This , I presume, will not cause any segmentation fault.

And just a small advice. don't mix-up the OOP and C. Otherwise, you will keep facing such kind of problems.

comments powered by Disqus