Deallocating 2D array in C

Question

For whatever reason I am getting the following error when trying to free the 2D array I created:

Error in `./a.out': free(): invalid next size (fast): 0x0000000001759310 *** Aborted (core dumped)

I printed out the contents of the array and they are correct and I am able access all of the elements. However, I am yet to be able to free it. The error occurs when going through the freeing loop, i.e. freeing double *. Would appreciate any help.

Thanks!

Here is the code:

/*allocation*/
double **block_mat = (double **) malloc (sizeof(double *) * num_blocks);
int i;
for (i = 0; i <num_blocks; i++){
    block_mat[i] = (double *) malloc (sizeof(double) * s);
}

/*freeing*/
for (i = 0; i < num_blocks; i++){
    free(block_mat[i]);
}

free(block_mat);

EDIT: The error was found! I under-allocated memory...So when I printed out my arrays they looked like everything was fine... I allocated arrays of sizes s, but used s^2 instead. Thank you everyone!

Answer 1

Are you sure that these pointers were all allocated with distinct mallocs?

free(my_matrix->vals);
free(my_matrix->cols);
free(my_matrix->rows);
free(my_matrix);
for (i = 0; i < num_blocks; i++){
    free(block_mat[i]);
}
free(block_mat);

If two of them are the same pointer, then you are trying to free something twice.

Answer 2

You allocate space for s doubles for each block_mat[i]. Later, you access

block_mat[i][block_index] = 0;
block_index++;

but you never check that block_index goes out of bounds, which it does.

If you write beyond the s allocated doubles, you might corrupt the internal control data for the subsequent pointer, which usually is placed before the pointer returned by malloc and which is required to be intact by free.

Answer 3

This is incorrect:

free(my_matrix);
free(my_matrix->vals);
free(my_matrix->cols);
free(my_matrix->rows);

The order should be at least like this:

free(my_matrix->vals);
free(my_matrix->cols);
free(my_matrix->rows);
free(my_matrix);

Answer 4

You have several problem that can impact allocation. The first, what if the call to strtol fails?

    s = (int)strtol(argv [3], &ptr, 10);

You should check the conversion:

#include <errno.h>
...
else {
    errno = 0;
    long tmp = strtol(argv [3], &ptr, 10);
    if ((errno == ERANGE && (tmp == LONG_MIN || tmp == LONG_MAX)) ||
        (errno != 0 && tmp == 0)) {
        perror ("strtol");
        exit (EXIT_FAILURE);
    }

    if (ptr == argv[3]) {
        fprintf (stderr, "No digits were found\n");
        exit (EXIT_FAILURE);
    }
    s = (int)tmp;
}

Next, with your allocation and free, you were on the right track, but don't cast the return of malloc. Casting the return only increases the chance of a hard to debug error. When allocating a pointer-to-pointer-to-double, you need to allocate as many pointers as needed. (num_blocks) in your case. When computing the size of the allocation, you can simply dereference the variable being allocated which again lessens the chance of error:

int i;
double **block_mat = malloc (sizeof *block_mat * num_blocks);
/* check allocation for failure */

for (i = 0; i <num_blocks; i++){
    block_mat[i] = malloc (sizeof **block_mat * s);
    /* check allocation for failure */
}

/*freeing*/
for (i = 0; i < num_blocks; i++){
    free (block_mat[i]);
}
free(block_mat);

Note: you must initialize all values in block_mat before you begin referencing the elements to prevent the deference of an unassigned value later. (e.g. double result = block_mat[i][j] * Pi;) Consider adding:

    block_mat[i] = malloc (sizeof **block_mat * s);
    /* check for failure */
    block_mat[i] = 0.0;

Note: When allocating numerical matricies, consider using calloc instead of malloc as it will initialize all values to '0' and prevent the inadvertent dereference of an unassigned valued -- which is undefined behavior. Using calloc you would have:

int i;
double **block_mat = calloc (num_blocks, sizeof *block_mat);
/* check allocation for failure */

for (i = 0; i <num_blocks; i++){
    block_mat[i] = calloc (s, sizeof **block_mat);
    /* check allocation for failure */
}

/*freeing*/
for (i = 0; i < num_blocks; i++){
    free (block_mat[i]);
}
free(block_mat);

Note: when s is greater than 1, you will alloc space for more than one double at each pointer. Noting prevents you from doing that, but you are responsible for handling the offset to each value. (e.g. result = *(block_mat[i] + j + n * sizeof **block_mat) * Pi;, where n is 0 <= n < s) If you are storing multiple doubles in each pointer, consider using a pointer-to-struct instead. E.g.:

typedef struct point {
    double x;
    double y;
    double z;
} point;