structs free up memory function - c

Question

I have a program i made and its running perfectly! the only problem is the free pointers function this is a link for the full code https://codeshare.io/aVE3n3

The problem is that i success to free the player name pointer, but after the program doesn't let me free the player's pointer. I'd love to get some help, thanks.

    void freeTeam(team* t,int size)
{
    int temp;
    for (int j = 0; j < size; j++)
    {
        temp = t[j].current_players;
        for (int i = 0; i < temp; i++)
        {
            free(t->players[i].name);
        }
        free(t->players);
        for (int i = 0; i < temp; i++)
        {
            free(t[i].team_name);
        }
        free(t[j]);
    }
}

@TarickWelling You can free an array of pointers if it is dynamically allocated. — MikeCAT, May 09 '20 at 10:55
Your `#include` list is wrong. You're missing `stdlib.h`, which is required for the proper prototypes for many of the functions you're using (such as `free` and `calloc`). You're also not using `calloc` correctly (you're only passing *one* argument; it requires *two*. Further, `free(t[j]);` can't possibly be right. It shouldn't even *compile*. `t[j]` is a `team`, not a `team*`. — WhozCraig, May 09 '20 at 10:56
@MikeCAT, was my guess on the code given not the linked code. Did remove — Tarick Welling, May 09 '20 at 10:57
I took a further look at that linked code. It's *loaded* with logical or outright compiler errors. double indirection where single is what should be use, wrong function arguments, etc. It looks like a series of unfortunate guesses, with many of them wrong. I suggest you think about what you're really trying to do. — WhozCraig, May 09 '20 at 11:06

score 1 · Answer 1 · answered May 09 '20 at 11:05

The first wrong part is

    t->players = (player**)calloc(t->max_players, sizeof(player*));

in initTeam().

t->players has type player* and its element type is player. In typical environment, player (one pointer and other elements) consume more memory than player* (one pointer), so you won't allocate enough memory here.

It should be

    t->players = calloc(t->max_players, sizeof(player));

or

    t->players = calloc(t->max_players, sizeof(*t->players));

(note: c - Do I cast the result of malloc? - Stack Overflow)

The second wrong part is the freeTeam function.

free(t->players[i].name); may cause double (or more) free because only t[0] is dealt with.
free(t[i].team_name); may cause double (or more) free and/or out-of-bounds read because the usage of loop is wrong.
free(t[j]); is invalid because structure is not a pointer.

It should be

void freeTeam(team* t,int size)
{
    int temp;
    for (int j = 0; j < size; j++)
    {
        temp = t[j].current_players;
        for (int i = 0; i < temp; i++)
        {
            free(t[j].players[i].name);
        }
        free(t[j].players);
        free(t[j].team_name);
    }
}

after that, t should be freed after freeTeam(t,size); in main().

Additionaly, you should use standard int main(void) in hosted environment instead of void main(), which is illegal in C89 and implementation-defined in C99 or later, unless you have some special reason to use non-standard signature.

Worth noting, the OP will eventually encounter a double-free on the player name field even *after* fixing all of the above. They shallow-copy the player structure in the build-loop of `nameLikeA`, so two `player` records (one in the input team, one in the result team), will have `name` members that point to the same memory. Once `freeTeam` is called on one, the other is left with dangling pointer(s). — WhozCraig, May 09 '20 at 11:08

structs free up memory function - c

1 Answers1