Search code examples
cmatrixstructmallocfree

How would I free a struct?


I have a main that takes in a file and adds the numbers within the file to the matrix. I have everything working and it prints out the right answers, but in valgrind I'm getting a leak, I for sure know its because I am not freeing MatC in the loop: else if (numOfMatrices > 2). The lines with the arrows in front of it are the lines that need to be free'd.

Here's the main function:

#define _GNU_SOURCE
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include "mmult.h"

int main(int argc, char* argv[]){
    (void) argc;
    FILE* fp = fopen(argv[0], "r"); 
    char *firstLine = NULL;
    char *ptr;
    if (!fp){
        fprintf(stderr, "Can't Open File");
        return EXIT_FAILURE;
    } else{ // Gets the number of matrices
        char *buf = NULL;
        size_t len = 100;
        while (getline(&buf, &len, stdin)){
            char *pch;
            pch = strtok(buf, " \n");
            firstLine = buf;
            (void) pch;
            break;
        }
    }

    int numOfMatrices = strtod(firstLine, &ptr);
    free(firstLine);

    // Gets the number of rows and columns of the matrix
    for (int m = 0; m < numOfMatrices; m++){
        int x, y, i, j;
        Matrices MatA;
        Matrices MatB;
        Matrices MatC;
        Matrices MatD;
        if (numOfMatrices == 1){
            MatA.Matrix = mread(stdin, &x, &y);
            MatA.row = x;
            MatA.col = y;

            printf("%d %d\n", MatA.row, MatA.col);
            mwrite(stdout, MatA.row, MatA.col, MatA.Matrix);
            xfree(MatA.row, MatA.col, MatA.Matrix);
        } else if (numOfMatrices >= 2){
            if (numOfMatrices == 2){
                if (m == 0){
                    MatA.Matrix = mread(stdin, &x, &y);
                    MatA.row = x;
                    MatA.col = y;

                    MatB.Matrix = mread(stdin, &i, &j);
                    MatB.row = i;
                    MatB.col = j;

                    MatC.Matrix = mmult(MatA.row, MatA.col, MatA.Matrix, MatB.row, MatB.col, MatB.Matrix);
                    MatC.row = MatA.row;
                    MatC.col = MatB.col;

                    xfree(MatA.row, MatA.col, MatA.Matrix);
                    xfree(MatB.row, MatB.col, MatB.Matrix);
                    printf("%d %d\n", MatC.row, MatC.col);
                    mwrite(stdout, MatC.row, MatC.col, MatC.Matrix);
                    xfree(MatC.row, MatC.col, MatC.Matrix);
                }
             } else if (numOfMatrices > 2){
                if (m == 0){
                    MatA.Matrix = mread(stdin, &x, &y);
                    MatA.row = x;
                    MatA.col = y;

                    MatB.Matrix = mread(stdin, &i, &j);
                    MatB.row = i;
                    MatB.col = j;

                ->  MatC.Matrix = mmult(MatA.row, MatA.col, MatA.Matrix, MatB.row, MatB.col, MatB.Matrix);
                    MatC.row = MatA.row;
                    MatC.col = MatB.col;

                    xfree(MatA.row, MatA.col, MatA.Matrix);
                    xfree(MatB.row, MatB.col, MatB.Matrix);

                    for (int x = 2; x < numOfMatrices; x++){
                        MatD.Matrix = mread(stdin, &i, &j);
                        MatD.row = i;
                        MatD.col = j;

                    ->  MatC.Matrix = mmult(MatC.row, MatC.col, MatC.Matrix, MatD.row, MatD.col, MatD.Matrix);

                        xfree(MatD.row, MatD.col, MatD.Matrix);
                        if (x == (numOfMatrices-1)){
                            printf("%d %d\n", MatC.row, MatC.col);
                            mwrite(stdout, MatC.row, MatC.col, MatC.Matrix);
                            xfree(MatC.row, MatC.col, MatC.Matrix);
                        }

                    }
                } 
            }
        }
    }
    fclose(fp);
}

Also, Here's my xfree function that free's a matrix:

// Free all memory allocated for A 
void xfree(int r, int c, double **A){
    (void) c;
    for (int y = 0; y < r; y++){
        free(A[y]);
    }
    free(A);
}

I also created a struct for the Matrices:

typedef struct Matrices{
        double **Matrix;    // The matrix held in this node
        int row;            // Number of rows in the Matrix
        int col;            // Number of Columns in the Matrix
    } Matrices;

Valgrind Output:

ShaolinGOD@comp:~/Desktop/Project2$ valgrind --leak-check=full --show-leak-kinds=all --track-origins=yes ./mmult < OneMatrix.txt
==7984== Memcheck, a memory error detector
==7984== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
==7984== Using Valgrind-3.11.0 and LibVEX; rerun with -h for copyright info
==7984== Command: ./mmult
==7984== 
2 2
   52.00    52.00 
   80.00    80.00 
==7984== 
==7984== HEAP SUMMARY:
==7984==     in use at exit: 144 bytes in 9 blocks
==7984==   total heap usage: 30 allocs, 21 frees, 133,624 bytes allocated
==7984== 
==7984== 32 bytes in 2 blocks are indirectly lost in loss record 1 of 4
==7984==    at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==7984==    by 0x400D11: xalloc (mmult.c:83)
==7984==    by 0x400BF7: mmult (mmult.c:65)
==7984==    by 0x401111: main (mmult-driver.c:86)
==7984== 
==7984== 48 (16 direct, 32 indirect) bytes in 1 blocks are definitely lost in loss record 2 of 4
==7984==    at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==7984==    by 0x400CDE: xalloc (mmult.c:81)
==7984==    by 0x400BF7: mmult (mmult.c:65)
==7984==    by 0x401111: main (mmult-driver.c:86)
==7984== 
==7984== 64 bytes in 4 blocks are indirectly lost in loss record 3 of 4
==7984==    at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==7984==    by 0x400D11: xalloc (mmult.c:83)
==7984==    by 0x400BF7: mmult (mmult.c:65)
==7984==    by 0x4011A4: main (mmult-driver.c:98)
==7984== 
==7984== 96 (32 direct, 64 indirect) bytes in 2 blocks are definitely lost in loss record 4 of 4
==7984==    at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==7984==    by 0x400CDE: xalloc (mmult.c:81)
==7984==    by 0x400BF7: mmult (mmult.c:65)
==7984==    by 0x4011A4: main (mmult-driver.c:98)
==7984== 
==7984== LEAK SUMMARY:
==7984==    definitely lost: 48 bytes in 3 blocks
==7984==    indirectly lost: 96 bytes in 6 blocks
==7984==      possibly lost: 0 bytes in 0 blocks
==7984==    still reachable: 0 bytes in 0 blocks
==7984==         suppressed: 0 bytes in 0 blocks
==7984== 
==7984== For counts of detected and suppressed errors, rerun with: -v
==7984== ERROR SUMMARY: 2 errors from 2 contexts (suppressed: 0 from 0)
ShaolinGOD@comp:~/Desktop/Project2$ 

Solution

  • The xfree function is okay. The problem is that xfree is not called for all allocations of MatC.Matrix

    It seems you are trying to convert MatC.Matrix. To do so, you can allocate a temporary matrix, copy the old matrix to temporary, free the old matrix, then set old matrix pointer to the temporary matrix.

    Example, change the following section:

    MatC.Matrix = mmult(MatA.row, MatA.col, MatA.Matrix, MatB.row, MatB.col, MatB.Matrix);
    MatC.row = MatA.row;
    MatC.col = MatB.col;
    
    xfree(MatA.row, MatA.col, MatA.Matrix);
    xfree(MatB.row, MatB.col, MatB.Matrix);
    
    for (int x = 2; x < numOfMatrices; x++) 
    {
        MatD.Matrix = mread(stdin, &i, &j);
        MatD.row = i;
        MatD.col = j;
    
        MatC.Matrix = mmult(MatC.row, MatC.col, MatC.Matrix, MatD.row, MatD.col, MatD.Matrix);
        xfree(MatD.row, MatD.col, MatD.Matrix);
    
        if (x == (numOfMatrices - 1)) 
        {
            printf("%d %d\n", MatC.row, MatC.col);
            mwrite(stdout, MatC.row, MatC.col, MatC.Matrix);
            xfree(MatC.row, MatC.col, MatC.Matrix);
        }
    }
    

    To this:

    MatC.Matrix = mmult(MatA.row, MatA.col, MatA.Matrix, MatB.row, MatB.col, MatB.Matrix);
    MatC.row = MatA.row;
    MatC.col = MatB.col;
    
    xfree(MatA.row, MatA.col, MatA.Matrix);
    xfree(MatB.row, MatB.col, MatB.Matrix);
    
    for (int x = 2; x < numOfMatrices; x++) 
    {
        MatD.Matrix = mread(stdin, &i, &j);
        MatD.row = i;
        MatD.col = j;
    
        Matrices temp;
        temp.Matrix = mmult(MatC.row, MatC.col, MatC.Matrix, MatD.row, MatD.col, MatD.Matrix);
        xfree(MatD.row, MatD.col, MatD.Matrix);
    
        //free MatC.Matrix
        xfree(MatC.row, MatC.col, MatC.Matrix);
    
        //copy pointers from temp to MatC.Matrix
        MatC.Matrix = temp.Matrix;
        for (int k = 0; k < temp.row; k++) 
            MatC.Matrix[k] = temp.Matrix[k];
        MatC.row = temp.row;//<== added (EDIT ***********)
        MatC.col = temp.col;//<== added (EDIT ***********)
    
        if (x == (numOfMatrices - 1)) 
        {
            printf("%d %d\n", MatC.row, MatC.col);
            mwrite(stdout, MatC.row, MatC.col, MatC.Matrix);
        }
    }
    
    //add xfree here:
    xfree(MatC.row, MatC.col, MatC.Matrix);
    

    Now the number of allocations and xfree should match.