Valgrind Memory Leaks, Invalid free()

Question

typedef struct {
   int **a;
   int **b;
   int **c;
   int i;
} test_t;

test_t *create(int i) {
    test_t *test = malloc(i * sizeof(test_t));

    test->i = i;

    test->c = malloc(i * sizeof(int *));
    for (int j = 0; j < i; ++j) {
        test->c[j] = malloc(sizeof(int *));
    }

    test->a = malloc(sizeof(int *));
    test->a = &(test->c[0]);
    test->b = malloc(sizeof(int *));
    test->b = &(test->c[0]);
    return test;
}

void delete(test_t *test) {
    free(test->a);
    free(test->b);
    for (int i = 0; i < test->i; ++i)
        free(test->c[i]);
    free(test->c);
    free(test);
}

int main() {
    test_t *test;

    test = create(3);

    delete(test);

    return 0;
}

What's wrong with this code?

When I run Valgrind, I get 5 errors and some memory leaks.

I don't see any memory leak, do you?

I get errors like:

Invalid free() / delete / delete[] / realloc()
Address 0x4a380e0 is 0 bytes inside a block of size 24 free'd
Block was alloc'd at
Invalid read of size 8

Could anybody help me with that, please?

P.S. The code works fine, but it has memory leaks, so it doesn't.

'test->a' gets malloced, but then overwritten in the next line. Same with test->b. — Martin James
test->a = malloc(sizeof(int *)); test->a = &(test->c[0]); What do you think that does? The second expression overwrites the malloc result from the first. That results in a memory leak. And when you free test->a it will be somewhere inside c. Which is exactly what valgrind tells you. — kaylum
@kaylum Hmmm. So, test->a = some-address overwrites test->a address? When I try to printf that, it is not overwritten. That's weird. I'd like to store an address of a first item in array to test->a and test->b. — John Doe

Schwern Schwern · Accepted Answer · 2020-11-27T04:24:20

Eyeballing it...

    test->a = malloc(sizeof(int *));
    test->a = &(test->c[0]);
    test->b = malloc(sizeof(int *));
    test->b = &(test->c[0]);

Since test->a and test->b are immediately reassigned, the mallocs have no effect except to leak memory. The above should be simply...

    test->a = &(test->c[0]);
    test->b = &(test->c[0]);

With that the rule of thumb that the number of mallocs should equal the number of frees works. Three mallocs, one in a loop. Three frees, one in a loop.

void delete(test_t *test) {
    for (int i = 0; i < test->i; i++) {
        free(test->c[i]);
    }
    free(test->c);
    free(test);
    test->a = NULL;
    test->b = NULL;
}

test->a and test->b should not be freed as they are borrowing test->c[0]'s memory. We need too avoid freeing it twice. Since that borrowed memory is invalid and can no longer be used, we set it to NULL as a precaution.

Valgrind Memory Leaks, Invalid free()

1 Answers