Search code examples
cdynamic-memory-allocation

How to properlly allocate memory for structures in C?


I dont know how to allocate memory for a struct in my C program without it segmentation faulting.

typedef struct Player{
      char *name,  *surname;
      int xp;
} Player;

typedef struct Team{
      char *name;
      Player *player;
} Team;

typedef struct List
{
    Team *team;
    struct List *next;
} List_Node;

I need to make a list of teams with an array of players but for my program to not Segfault I need to malloc every member of every structure. Then I have to free every member of every structure. It is a resonably big project and I get heap corruption errors and I dont know if I initialize my structures correctly.

With the pointer initialized in main.

List_Node *list_node = (List_Node*)malloc(sizeof(List_node));

And the function

void create_list_Node(List_Node *temp)
{
    temp->team = malloc....
    temp->team->name = malloc....
    temp->team->player = malloc....
    temp->team->player->name = malloc....
    temp->team->player->surname = malloc....
    temp->next = NULL;
}

Called with the node as a parameter.


Solution

  • To reduce "many pieces of memory scattered all over the place" (and the hassle of keeping track of where all the little pieces are allocated/freed) you can allocate a single piece of memory for multiple things (e.g. a structure plus a string). For example:

    typedef struct Player{
          char *name,  *surname;
          int xp;
    } Player;
    
    typedef struct Team{
          struct Team *next;
          char *name;
          Player *player;
    } Team;
    
    Player *createPlayer(char *name, char *surname, int xp) {
        Player *newPlayer;
        int len1, len2;
    
        len1 = strlen(name);
        len2 = strlen(surname);
        newPlayer = malloc(sizeof(Player) + len1+1 + len2+1);
        if(newPlayer != NULL) {
            newPlayer->next = NULL;
            newPlayer->name = (char *)newPlayer + sizeof(Player);
            memcpy(newPlayer->name, name, len1+1);
            newPlayer->surname = newPlayer->name + len1+1;
            memcpy(newPlayer->surname, surname, len2+1);
            plyer->xp = xp;
        }
        return newPlayer;
    }
    
    Team *createTeam(Player *player, char *name) {
        Team *newTeam;
        int len1;
    
        len1 = strlen(name);
        newTeam = malloc(sizeof(Team) + len1+1);
        if(newTeam != NULL) {
            newTeam->next = NULL;
            newTeam->name = (char *)newTeam + sizeof(Team);
            memcpy(newTeam->name, name, len1+1);
            newTeam->player = player;
        }
        return newTeam;
    }
    

    This also means that it's easy to free anything - e.g. you can free(player) without messing about with the individual fields.

    Note: I got rid of List_Node and put a next field in the Team structure. This helps to improve performance for iteration (and also reduces excessive malloc/free). To understand this, imagine you're listing all team names and do something like this:

        while(node != NULL) {
            printf("name: %s\n", node->team->name)
            node = node->next;
        }
    

    In this case; CPU stalls while it waits for node to get fetched from memory, then stalls while it waits for node->team to get fetched from memory. Now imagine this:

        while(team != NULL) {
            printf("name: %s\n", team->name)
            team = team->next;
        }
    

    In this case; CPU stalls once instead of twice so it's faster to iterate through the list.