Question

I'm having some issues with producing an int matrix without creating memory leaks. I want to be able to make a given (global) matrix into any size dynamically via read_matrix(). But then i want to be able to free the memory later on. So in my main method the second printf should result in a bus error since it should not have any memory allocated to it. How would i go about creating this?

int**       first_matrix;
int**       second_matrix;
int**       result_matrix;

int** read_matrix(int size_x, int size_y)
{
    int** matrix;
    matrix = calloc(size_x, sizeof(int*));
    for(int i = 0;i<size_x;i++) {
        matrix[i] = calloc(size_y, sizeof(int));
    }
    for(int i = 0;i<size_x;i++) {
        for(int j = 0;j<size_y;j++) {
            matrix[i][j] = i*10+j;
        }
    }
    return matrix;
}

int main(int stackc, char** stack)
{
    first_matrix = read_matrix(10,10);
    printf("9:3 %d - 4:6 %d \n", first_matrix[9][3], first_matrix[4][6]);
    free(*first_matrix);
    free(first_matrix);
    printf("9:3 %d - 4:6 %d \n", first_matrix[9][3], first_matrix[4][6]);
}
Was it helpful?

Solution

Just because the memory has been free'd doesn't mean you can't access it! Of course, it's a very bad idea to access it after it's been free'd, but that's why it works in your example.

Note that free( *first_matrix ) only free's first_matrix[0], not the other arrays. You probably want some kind of marker to signify the last array (unless you will always know when you free the outer array how many inner arrays you allocated). Something like:

int** read_matrix(int size_x, int size_y)
{
    int** matrix;
    matrix = calloc(size_x, 1+sizeof(int*)); // alloc one extra ptr
    for(int i = 0;i<size_x;i++) {
        matrix[i] = calloc(size_y, sizeof(int));
    }
    matrix[size_x] = NULL; // set the extra ptr to NULL
    for(int i = 0;i<size_x;i++) {
        for(int j = 0;j<size_y;j++) {
            matrix[i][j] = i*10+j;
        }
    }
    return matrix;
}

Then when you're freeing them:

// keep looping until you find the NULL one
for( int i=0; first_matrix[i] != NULL; i++ ) {
    free( first_matrix[i] );
}
free( first_matrix );

OTHER TIPS

You need to free each row individually:


void free_matrix(int **matrix, int size_x)
{
    for(int i = 0; i < size_x; i++)
        free(matrix[i]);
    free(matrix);
}

Freeing the memory doesn't make it go away, it just means that another allocation might grab that same chunk of memory. Whatever you put in it will still be there until something else overwrites it.

Also, you're not freeing everything you allocated. You're only freeing the array of pointers and the first row. But even if you free everything correctly, you would still have the same effect.

If you want to create a "bus error" you need to point to memory that doesn't belong to your process. Why do you want to do that anyway?

You only freed the first row (or column) of first_matrix. Write another function like this:

void free_matrix(int **matrix, int rows)
{
    int i;
    for(i=0; i<rows; i++)
    {
        free(matrix[i]);
    }
    free(matrix);
}

You might want to make the matrix into a struct to store it's row and column count.

I recommend using valgrind to track down unfree'd memory, as opposed to trying to make a bus error occur. It rocks for lots of other stuff as well.

Sam

You're getting memory leaks because you're freeing the first row of the matrix and the list of rows, but none of the 1 to nth rows. You need to call free in a loop.

There are a couple of alternatives, however: - Allocate sizeof(int*)rows + rowscols*sizeof(int) bytes and use the first bytes for the row pointers. That way, you only have a single chunk of memory to free (and it's easier on the allocator, too) - Use a struct that contains the number of rows. Then you can avoid the row list altogether (saving memory). The only downside is that you have to use a function, a macro, or some messy notation to address the matrix.

If you go with the second option, you can use a struct like this in any C99 compiler, and again only have to allocate a single block of memory (of size numints*sizeof(int)+sizeof(int)):

struct matrix {
    int rows;
    int data[0];
}

The concept you are missing here, is that for every calloc, there must be a free. and that free must be applied to the pointer passed back from calloc.

I recommend you create a function (named delete_matrix) that uses a loop to free all of the pointers that you allocate in here

for(int i = 0;i < size_x;i++) { matrix[i] = calloc(size_y, sizeof(int)); }

then, once that is done, free the pointer allocated by this.

matrix = calloc(size_x, sizeof(int*));

The way you are doing it now,

free(*first_matrix); free(first_matrix);

won't do what you want it to do.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top