agility-treads
agility-treads

Reputation: 3

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.

Upvotes: 0

Views: 1095

Answers (2)

Brendan
Brendan

Reputation: 37214

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.

Upvotes: 1

Mikpa
Mikpa

Reputation: 1922

If we use this function to create an empty node:

void create_list_Node(List_Node *temp)
{
    temp->team = calloc(1, sizeof(Team));
    temp->team->player = calloc(1, sizeof(Player));
    temp->next = NULL;
}

How you allocate the namn:s and surname depends on where these strings get from. If you get an allocated pointer that no other code will free, you can just "adopt" the pointer:

char *mystring;
... code that allocates
temp->team->name = mystring;

If you have a local string array, just dup it:

char local[100];
... code that fill it, maybe gets()
temp->team->name = strdup(local);

Don't forget to free strings and structures!

Upvotes: 1

Related Questions