How to avoid memory leak when passing function-returned pointer as input to other function?

Question

I'm trying to write a simple C matrix library that dynamically allocates the memory required and that allows to "nest" function calls (like fun1(fun2(x))) in order to avoid declaring too many temporary variables when performing long operations. However I cannot get rid of the memory leak caused by never freeing the structs created inside the functions. The example below clearly shows that. Any suggestion on how to solve that without declaring other temporary variables?

Thanks a lot

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

struct _MatrixF{
    uint8_t rows;
    uint8_t cols;
    float* mat;
};
typedef struct _MatrixF* Matrix;

Matrix newMatrix(uint8_t rows, uint8_t cols){
    Matrix matrix = malloc(sizeof(struct _MatrixF));
    matrix->rows = rows;
    matrix->cols = cols;
    matrix->mat = malloc(rows * cols * sizeof(float));
    return matrix;
}

Matrix matIdentity(uint8_t rows, uint8_t cols){
    Matrix matrix = newMatrix(rows, cols);
    uint16_t ii;
    for (ii = 0; ii < (cols * rows); ii++)
        matrix->mat[ii] = (((ii / cols) == (ii % cols))? 1.0f : 0.0f);
    return matrix;
}


Matrix matAdd(Matrix lhs, Matrix rhs){
    Matrix m = newMatrix(lhs->rows, lhs->cols);
    uint16_t ii;
    for (ii = 0; ii < (lhs->cols * lhs->rows); ii++) {
        m->mat[ii] = lhs->mat[ii] + rhs->mat[ii];
    }
    return m;
}

Matrix matMult(Matrix lhs, Matrix rhs){
    uint8_t i, j, k;
    Matrix result = newMatrix(lhs->rows, rhs->cols);
    for (i=0; i < lhs->rows; i++)
        for (j=0; j < rhs->cols; j++)
            for (k=0; k < lhs->cols; k++)
                MAT(result, i, j) += MAT(lhs, i, k) * MAT(rhs, k, j);
    return result;
}

void mprint(Matrix m){
    printf("%dx%d\n", m->rows, m->cols);
    for(int i=0; i<m->rows; i++) {
        for (int j = 0; j<m->cols; j++)
            printf("%4.6f\t",(float) m->mat[(i) * m->cols + (j)]);
        printf("\n");
    }
}

int main(int argc, const char * argv[]) {
    Matrix A = matIdentity(4, 3);
    Matrix B = matIdentity(3, 2);
    Matrix C = matIdentity(2, 4);
    Matrix D = matIdentity(4, 4);
    uint16_t len = 64000;
    while (len--){
        Matrix E = matAdd(D, matAdd(D, matMult(matMult(A, B),C)));
        mprint(matMult(A, B));
        mprint(A);
        mprint(E);
    }
    return 0;
}

Answer 1

C does not provide much help in dealing with this automatically. You can store pointers to temporary objects, and free them at the end of each iteration:

while (len--){
    Matrix t1, t2, t3, t4;
    Matrix E = matAdd(D, t1=matAdd(D, t2=matMult(t3=matMult(A, B),C)));
    mprint(t4=matMult(A, B));
    mprint(A);
    mprint(E);
    matFree(t1);
    matFree(t2);
    matFree(t3);
    matFree(t4);
}

Answer 2

If the matrices are small, pass them by value. You will have to define the size statically and prepare different structure and set of functions for each (useful) size. This will be much faster (no allocations, no pointer chasing) and safer (no leaks, no use after free, no null pointers).

struct mat4 {
    float mat[4*4];
};

struct mat4 matIdentity4(void) {
    struct mat4 identity = {
        1,0,0,0,
        0,1,0,0,
        0,0,1,0,
        0,0,0,1,
    };
    return identity;
}

...

struct mat3 {
    float mat[3*3];
};

...

If the size is too variable but still pretty small, you can still pass them by value. You just need to pick an upper bound of width and height:

enum {
    MAX_MATRIX_SIZE = 16,
};

struct Matrix {
    int rows;
    int cols;
    float mat[MAX_MATRIX_SIZE*MAX_MATRIX_SIZE];
};

If the matrices are huge, You don't want to allocate and copy them willy-nilly. An unexpected temporary matrix might put you over available memory limit. Let the user explicitly allocate and free them. You probably also want your operations to mutate one of their arguments (to safe memory, safe cache and ease cleaning up):

struct Matrix *result = matNew(10,10);
struct Matrix *tmp = matNew(10,10);
matSetToIdentity(result);
fun1(tmp);
matAdd(result, tmp);
fun2(tmp);
matAdd(result, tmp);
matPrint(result);
matFree(tmp);
matFree(result);

If memory is not a problem, you can keep references to allocated matrices and free them all at once when they are no longer needed:

struct Matrix{
    struct Matrix* next; // List embedded in elements.
                         // See: https://isis.poly.edu/kulesh/stuff/src/klist/
    int rows;
    int cols;
    float mat[];
};

struct MatrixPool {
    struct Matrix* last;
};

struct Matrix *matIdentity(struct MatrixPool *pool, int rows, int cols) {
    struct Matrix* matrix = malloc(sizeof(struct Matrix)
                                    +sizeof(float)*rows*cols);
    matrix->next = pool->last;
    matrix->rows = rows;
    matrix->cols = cols;
    matrix->mat = ...;
    pool->last = matrix;
    return matrix;
}

void matrixPoolFree(struct MatrixPool *pool) {
    while(pool->last != NULL) {
        struct Matrix* matrix = pool->last;
        pool->last = matrix->next;
        free(matrix);
    }
}

int main() {
    struct MatrixPool pool = {0};
    struct Matrix* x = matIdentity(&pool, 10, 10);
    struct Matrix* y = fun1(&pool, fun2(&pool, x));
    mprint(y);
    matrixPoolFree(&pool);
}

Next two options are mentioned for educational purposes only. Please do not ever use them in real code. It's much saner to use language with garbage collector (Java, Python) or RAII (C++, Rust).

If you don't want to define and pass pool everywhere, you can kinda automate that using macros and alloca (or probably variable length arrays):

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

struct Matrix{
    int rows;
    int cols;
    float mat[];
};

#define allocaMatrix(w, h) alloca(sizeof(struct Matrix)+w*h*sizeof(float))

struct Matrix *aux_matIdentity(struct Matrix *mat, int w, int h) {
    mat->rows = w;
    mat->cols = h;
    for(int y=0;y<mat->cols;y++) {
        for(int x=0;x<mat->rows;x++) {
            mat->mat[y*mat->rows+x] = x==y;
        }
    }
    return mat;
}

#define matIdentity(w,h) aux_matIdentity(allocaMatrix(w,h),w,h)

struct Matrix *aux_matAdd(struct Matrix *result,struct Matrix *left, struct Matrix *right) {
    result->rows = left->rows;
    result->cols = left->cols;
    for(int y=0;y<result->cols;y++) {
        for(int x=0;x<result->rows;x++) {
            result->mat[y*result->rows+x] = left->mat[y*result->rows+x] + right->mat[y*result->rows+x];
        }
    }
    return result;
}

#define matAdd(left,right) (aux_matAdd(allocaMatrix(left->rows,left->cols), left, right))

void matPrint(struct Matrix *mat) {
    for(int y=0;y<mat->cols;y++) {
        for(int x=0;x<mat->rows;x++) {
            printf("%.1f%c",mat->mat[y*mat->rows+x], x==mat->rows-1?'\n':' ');
        }
    }
}

int main() {
    int n = 16;
    struct Matrix *mat1 = matIdentity(n,n);
    struct Matrix *mat2 = matIdentity(n,n);
    matPrint(matAdd(mat1, mat2));
}

This will save you some keystrokes but the lifetime of those objects becomes really unobvious. You cannot return then from a function and if you allocate a lot of them (e.g. in a loop) or just a single big one, you can smash the stack.

If you want to really freely pass those pointers around, you can bastardize garbage collector into C.