Search code examples
cmemory-leaksmallocfreerealloc

Memory leak in C, 2d array in struct


I really need some help with memory leaks. Ive been staring at my code for a while and still cant find out where are those leaks. I think I am freeing everything, even sometimes unnecessarily, but there is still something.

Code

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

struct Matrix{
    int **data;
    int rows;
    int columns;
};

struct Matrix matrix_op(struct Matrix, struct Matrix, char);
int** _2d_array(int,int);
void print_matrix(struct Matrix);
void _2d_array_free(int**, int);
void _scalar_mul(struct Matrix*, int);

int** _2d_array(int x,int y){
    int** arr;
    arr = malloc(sizeof(int*)*x);
    for(int i = 0; i<x;++i){
        arr[i] = malloc(sizeof(int)*y);
    }
    return arr;
}
void _scalar_mul(struct Matrix* a, int s){
    for (int x = 0; x < a->rows; ++x) {
        for (int y = 0; y < a->columns; ++y)
            a->data[x][y] *= s;
    }
}

void _2d_array_free(int** array, int x){
    for (int i = 0; i < x; ++i) {
        free(array[i]);
    }
    free(array);
}

void exit_err(int err, char* message){
    fprintf(stderr,message);
    exit(err);
}

struct Matrix read_matrix(int x, int y){
    struct Matrix matrix;
    matrix.rows = x;
    matrix.columns = y;
    matrix.data = _2d_array(x,y);
    for (int i = 0; i < x; ++i) {
        for (int j = 0; j < y; ++j) {
            if(scanf("%d", &matrix.data[i][j])==0) exit_err(100,"Error...\n");
        }
    }
    return matrix;
}

struct Matrix matrix_op(struct Matrix a, struct Matrix b, char op) {
    struct Matrix res_matrix;
    int** result;
    if (op == '*') {
        if(a.columns==b.rows) {
            result = _2d_array(a.rows, b.columns);
            for (int x = 0; x < a.rows; ++x) {
                for (int y = 0; y < b.columns; ++y) {
                    int sum = 0;
                    for (int z = 0; z < a.columns; z++) {
                        sum += a.data[x][z] * b.data[z][y];
                    }
                    result[x][y] = sum;
                }
            }
        }
        else exit_err(100,"Error...\n");
        res_matrix.data = result;
        res_matrix.rows = a.rows;
        res_matrix.columns = b.columns;
    } else {
        if (op == '-') {
            _scalar_mul(&b,-1);
        }
        result = _2d_array(a.rows,a.columns);
        for (int x = 0; x < a.rows; ++x) {
            for (int y = 0; y < a.columns; ++y){
                result[x][y]=a.data[x][y]+b.data[x][y];
            }
        }
        res_matrix.data = result;
        res_matrix.rows = a.rows;
        res_matrix.columns = a.columns;
    }
    return res_matrix;
}

void print_matrix(struct Matrix matrix){
    printf("%d %d\n",matrix.rows, matrix.columns);
    for(int x = 0; x<matrix.rows;++x){
        for(int y = 0; y<matrix.columns;++y){
            if(y==0)printf("%d", matrix.data[x][y]);
            else printf(" %d",matrix.data[x][y]);
        }
        printf("\n");
    }
}

int main(){
    int x,y,count;
    count=0;
    char op='\0';
    struct Matrix* matrices = malloc(0);
    while(1){
        scanf("%d %d", &x, &y);
        matrices = realloc(matrices,sizeof(struct Matrix)*(++count));
        matrices[count-1]=read_matrix(x,y);
        if(op=='-') {
            _scalar_mul(&matrices[count - 1], -1);
        };
        if(op=='*') {
            struct Matrix result = matrix_op(matrices[count-2], matrices[count-1], op);
            _2d_array_free(matrices[count-1].data, matrices[count-1].rows);
            matrices[count-1] = result;
            _2d_array_free(matrices[count-2].data, matrices[count-2].rows);
            matrices[count-2].rows=0;
        }
        if(scanf("\n%c", &op)==EOF) break;

    }
    struct Matrix res;
    res.data = _2d_array(3,3);
    res.rows = 3;
    res.columns = 3;
    res = matrices[0];
    for (int i = 1; i < count; ++i) {
        if(res.rows>0&&matrices[i].rows>0) {
            struct Matrix tmp = matrix_op(res, matrices[i], '+');
            _2d_array_free(res.data, res.rows);
            res.rows = 0;
            res = tmp;
        }
        else if (res.rows==0){
            _2d_array_free(res.data, res.rows);
            res.rows = 0;
            res = matrices[i];
        }
    }
    print_matrix(res);
    _2d_array_free(res.data, res.rows);
    for (int j = 0; j < count; ++j) {
        if(matrices[j].rows!=0){
            _2d_array_free(matrices[j].data,matrices[j].rows);
        }
    }
    free(matrices);
}

Valgrind out:

==25== Invalid read of size 8
==25==    at 0x4008B5: _2d_array_free (in /mnt/c/Users/wM/ClionProjects/hw4/main)
==25==    by 0x401038: main (in /mnt/c/Users/wM/ClionProjects/hw4/main)
==25==  Address 0x51fc0d0 is 0 bytes inside a block of size 24 free'd
==25==    at 0x4C2BD57: free (vg_replace_malloc.c:530)
==25==    by 0x4008D7: _2d_array_free (in /mnt/c/Users/wM/ClionProjects/hw4/main)
==25==    by 0x400F51: main (in /mnt/c/Users/wM/ClionProjects/hw4/main)
==25==  Block was alloc'd at
==25==    at 0x4C2AC3D: malloc (vg_replace_malloc.c:299)
==25==    by 0x4007AC: _2d_array (in /mnt/c/Users/wM/ClionProjects/hw4/main)
==25==    by 0x400936: read_matrix (in /mnt/c/Users/wM/ClionProjects/hw4/main)
==25==    by 0x400D46: main (in /mnt/c/Users/wM/ClionProjects/hw4/main)
==25==
==25== Invalid free() / delete / delete[] / realloc()
==25==    at 0x4C2BD57: free (vg_replace_malloc.c:530)
==25==    by 0x4008BF: _2d_array_free (in /mnt/c/Users/wM/ClionProjects/hw4/main)
==25==    by 0x401038: main (in /mnt/c/Users/wM/ClionProjects/hw4/main)
==25==  Address 0x51fc130 is 0 bytes inside a block of size 28 free'd
==25==    at 0x4C2BD57: free (vg_replace_malloc.c:530)
==25==    by 0x4008BF: _2d_array_free (in /mnt/c/Users/wM/ClionProjects/hw4/main)
==25==    by 0x400F51: main (in /mnt/c/Users/wM/ClionProjects/hw4/main)
==25==  Block was alloc'd at
==25==    at 0x4C2AC3D: malloc (vg_replace_malloc.c:299)
==25==    by 0x4007DF: _2d_array (in /mnt/c/Users/wM/ClionProjects/hw4/main)
==25==    by 0x400936: read_matrix (in /mnt/c/Users/wM/ClionProjects/hw4/main)
==25==    by 0x400D46: main (in /mnt/c/Users/wM/ClionProjects/hw4/main)
==25==
==25== Invalid free() / delete / delete[] / realloc()
==25==    at 0x4C2BD57: free (vg_replace_malloc.c:530)
==25==    by 0x4008D7: _2d_array_free (in /mnt/c/Users/wM/ClionProjects/hw4/main)
==25==    by 0x401038: main (in /mnt/c/Users/wM/ClionProjects/hw4/main)
==25==  Address 0x51fc0d0 is 0 bytes inside a block of size 24 free'd
==25==    at 0x4C2BD57: free (vg_replace_malloc.c:530)
==25==    by 0x4008D7: _2d_array_free (in /mnt/c/Users/wM/ClionProjects/hw4/main)
==25==    by 0x400F51: main (in /mnt/c/Users/wM/ClionProjects/hw4/main)
==25==  Block was alloc'd at
==25==    at 0x4C2AC3D: malloc (vg_replace_malloc.c:299)
==25==    by 0x4007AC: _2d_array (in /mnt/c/Users/wM/ClionProjects/hw4/main)
==25==    by 0x400936: read_matrix (in /mnt/c/Users/wM/ClionProjects/hw4/main)
==25==    by 0x400D46: main (in /mnt/c/Users/wM/ClionProjects/hw4/main)
==25==
==25==
==25== HEAP SUMMARY:
==25==     in use at exit: 60 bytes in 4 blocks
==25==   total heap usage: 19 allocs, 19 frees, 432 bytes allocated
==25==
==25== LEAK SUMMARY:
==25==    definitely lost: 24 bytes in 1 blocks
==25==    indirectly lost: 36 bytes in 3 blocks
==25==      possibly lost: 0 bytes in 0 blocks
==25==    still reachable: 0 bytes in 0 blocks
==25==         suppressed: 0 bytes in 0 blocks
==25== Rerun with --leak-check=full to see details of leaked memory
==25==
==25== For counts of detected and suppressed errors, rerun with: -v
==25== ERROR SUMMARY: 7 errors from 3 contexts (suppressed: 0 from 0)

Input

3 7
-42 8 -62 0 7 46 3
7 11 45 89 -39 -27 70
-35 -5 0 -99 -50 -14 46
+
3 7
73 -88 -45 94 -41 -67 -66
0 22 30 29 72 -91 76
-95 -83 -46 -88 28 69 -62

Output

3 7
31 -80 -107 94 -34 -21 -63
7 33 75 118 33 -118 146
-130 -88 -46 -187 -22 55 -16

Output is ok, but the memory leak and valgrind errors are undesirable.

Thanks for your help!


Solution

  • One problem is this:

    struct Matrix res;
    res.data = _2d_array(3,3);
    res.rows = 3;
    res.columns = 3;
    res = matrices[0];
    

    You define res and initialize it, then you reassign it with matrices[0] which overwrite all the data you previously initialized, including the pointer to the data you just allocated.


    Also:

    _2d_array_free(res.data, res.rows);
    for (int j = 0; j < count; ++j) {
        if(matrices[j].rows!=0){
            _2d_array_free(matrices[j].data,matrices[j].rows);
        }
    }
    

    Here res.data points to some data allocated for one of the entries in matrices, so you are attempting to free the data twice which is wrong. The simple solution to this is to not call _2d_array_free for res.data.


    Lastly a little note about style and naming: You should really avoid using symbol-names with leading underscores. If you're not careful you might use a reserved symbol (underscore followed by an upper-case letter or another underscore). Leading underscores are usually used internally by the compiler and standard library.