Free Memory of 2d Array in struct-CodePudding

I'm writing a little game in C and I wanted to test it with Valgrind. Here is a little code example:

#include <stdio.h>
#include <stdlib.h>

typedef struct Game {
    int** field;
} Game;

void buildfield(Game* game, int length);
void printfield(Game* game, int length);
void freefield(Game* game, int length);

int main()
{
    struct Game* game = NULL;
    game = malloc(sizeof (struct Game));
    buildfield(game, 10);
    printfield(game, 10);
    freefield(game, 10);
    free(game);
    return 0;
}

void buildfield(Game* game, int length)
{
    game->field = (int**)malloc((sizeof (int*)) * 20);
    int i;
    for (i = 0; i < 20; i  ) {
        game->field[i] = (int*) malloc((sizeof (int)) * length);
    }

    for (int line = 0; line < length; line  ) {
        for (int col = 0; col < 81; col  ) {
            game->field[col][line] = 0;
        }
    }
}

void printfield(Game* game, int length)
{
    for (int i = 0; i < length; i  ) {
        printf("\n");
        for (int j = 0; j < 20; j  ) {
            printf("%d",game->field[i][j]);
        }
    }
}

void freefield(Game* game, int length)
{
    for (int i = 0; i < length;   i) {
        free(game->field[i]);
    }
    free(game->field);
}

Valgrind is saying:

HEAP SUMMARY:
==6239==     in use at exit: 968 bytes in 22 blocks
==6239==   total heap usage: 22 allocs, 0 frees, 968 bytes allocated

Why don't I free the allocations?

CodePudding user response：

Well in the function buildfield() You have alloted 20 blocks of memory

game->field = (int **) malloc(sizeof(int *) * 20);

Hence the most you can access by [] operator is game->field[19] But in the loop the program tries to access more blocks ahead of game->field[19] Causing a segmentation fault And the program crashes then and there. Without ever returning to main() let alone reach the free() statement. So it means your program never completed in the first place and crashed midway.

for (int line = 0; line < length; line  )
{
    for (int col = 0; col < 81; col  )  //when col becomes 20
    {
        game->field[col][line] = 0;     //this statement will try to access the non-alloted memory block when col is 20
    }
}

To check if your program crashes midway add some print statements at the end of the statements related to accessing memory. Because that's the most common source of runtime errors.

Also for this code try to keep in mind the maximum no. of memory blocks alloted that can be accessed by a specific pointer and change the condition of the marked for loop so that it will access memory within the allocation limit that is the no. of blocks you alloted using the malloc function.

The problem here is not the free() not working. But the segmentation fault. Assuming the code you provided is the exact replica of your code

CodePudding user response：

"Why don't I free the allocations?"

As noted in comments, there is extra complication in the code caused by use of unexplained values ( magic numbers ) to both create and free memory. For reasons explained in the link, among other problems, this can make matching count of frees with count of allocations difficult. The mis-match in times each of these is called in the reason Valgrind indicated blocks of memory remaining at the end of execution.

The following is your code with suggestions, including those specific to clearly applying the same number of calls to free() as to [m][c]alloc() (I chose to use calloc() here to avoid another loop (or memset()) to initialize memory.)

Note also, you may need to change the values that this example uses for #defines to meet your needs, but you only need to change them in one place, (at top of file.)

typedef struct Game {
    int** field;
} Game;

void buildfield(Game *game, int length);
void printfield(Game *game, int length);
void freefield(Game *game, int length);

#define COUNT  20//replace all magic numbers 
#define LENGTH 10//(change values of #defines to change shape and size of memory)

int main(void)//use a complete prototype for main
{
    struct Game* game = NULL;//pointer needs memory
    game = malloc(sizeof (struct Game));

    buildfield(game, LENGTH);
    printfield(game, LENGTH);
    freefield(game, LENGTH);
    free(game);
    return 0;
}

void buildfield(Game *game, int length)
{   //Note - not optimal to cast return of [c][m]alloc in C (only in C  )
    game->field = calloc(COUNT, sizeof(game->field));
    int i;
    for (i = 0; i < COUNT; i  ) {
        game->field[i] = calloc(LENGTH, (sizeof (game->field[i])) );
    }
}

void printfield(Game *game,int length)
{
    for (int i = 0; i < COUNT; i  ) {
        printf("\n");
        for (int j = 0; j < LENGTH; j  ) {
            printf("%d",game->field[i][j]);
        }
    }
}

void freefield(Game *game,int length)
{
    for (int i = 0; i < COUNT;   i) {
        free(game->field[i]);
    }
    free(game->field);
}