Search code examples
cmatrixmemory-leaksdynamic-memory-allocationfunction-definition

Sum of matrices in a returning matrix function


Below is a compilable program, most of it is made by Craig Estey that helped me with another question. I am just using it so people can test the program, but the problem I am facing is with a function to sum two matrices. Precisely, the function

Matrix* sum(Matrix* mat1, Matrix* mat2){
    int row, col, k, l;
    Matrix *mat3;
    mat3 = allocateMemory(mat1->row, mat1->col);

    if((mat1->row == mat2->row)&&(mat1->col == mat2->col)){ //verify if the two matrices have the same order
        for (row = 0; row < mat1->row; row++){
            for (col = 0; col < mat1->col; col++){
                k = row * mat1->col + col;//create index for matrix 1 element
                l = row * mat2->col + col;//create index for matrix 2 element
                mat3->a[k] = mat1->a[k] + mat2->a[l]; //sum elements and store in mat3
            }
        }
        return mat3;
    }
}

Compilable C program

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

typedef struct matrix {
    int row;
    int col;
    int *a;
} Matrix;

Matrix *allocateMemory(int row, int col)
{
    Matrix *mat = malloc(sizeof(*mat));

    if (mat == NULL) {
        perror("malloc");
        exit(1);
    }

    mat->row = row;
    mat->col = col;

    mat->a = calloc(row * col, sizeof(*mat->a));
    if (mat->a == NULL) {
        perror("calloc");
        exit(1);
    }

    return mat;
}

void printMatrix(Matrix *mat)
{
    int row, col, off;

    for (row = 0; row < mat->row; row++) {
        for (col = 0; col < mat->col; col++) {
            off = (row * mat->col) + col;
            printf("%d   ", mat->a[off]);
        }
        printf("\n");
    }
}

void matrix_fill(Matrix *mat)
{
    int row, col, off;
    int val = 1;

    for (row = 0; row < mat->row; row++) {
        for (col = 0; col < mat->col; col++) {
            off = (row * mat->col) + col;
            mat->a[off] = val++;
        }
    }
}

Matrix* sum(Matrix* mat1, Matrix* mat2){
    int row, col, k, l;
    Matrix *mat3;
    mat3 = allocateMemory(mat1->row, mat1->col);

    if((mat1->row == mat2->row)&&(mat1->col == mat2->col)){
        for (row = 0; row < mat1->row; row++){
            for (col = 0; col < mat1->col; col++){
                k = row * mat1->col + col;
                l = row * mat2->col + col;
                mat3->a[k] = mat1->a[k] + mat2->a[l];
            }
        }
        return mat3;
    }
}

int main(void){

   Matrix *m1 = allocateMemory(3,3);
   Matrix *m2 = allocateMemory(3,3);
   Matrix *m3 = allocateMemory(3,3);

    matrix_fill(m1);
    matrix_fill(m2);

    printf("\nMatrix 1\n");
    printMatrix(m1);

    printf("\nMatrix 2\n");
    printMatrix(m2);

    Matrix* sum(Matrix* m1, Matrix* m2);
    printf("\nSum of matrices m1 and m2\n");
    printMatrix(m3);

    return 0;
}

The sum function is supposed to return the result of the sum of two given matrices, but it is not working, just seems to return NULL value. But I couldn't understand why it is not working.


Solution

  • The function has undefined behavior and a memory leak because it can return nothing in the case when the condition in the if statement

    if((mat1->row == mat2->row)&&(mat1->col == mat2->col)){
    

    will evaluate to false.

    The function can be declared and defined the following way

    Matrix * sum( const Matrix *mat1, const Matrix *mat2 )
    {
        Matrix *mat3 = NULL;
    
        if ( ( mat1->row == mat2->row ) && ( mat1->col == mat2->col ) )
        {
            mat3 = allocateMemory( mat1->row, mat1->col );
          
            if ( mat3 != NULL )
            {
                for ( int row = 0; row < mat1->row; row++ )
                {
                    for ( int col = 0; col < mat1->col; col++ )
                    {
                        int k = row * mat1->col + col;
                        mat3->a[k] = mat1->a[k] + mat2->a[k];
                    }
                }
            }
        }
    
        return mat3;
    }
    

    Within the function main this allocation

    Matrix *m3 = allocateMemory(3,3);
    

    is redundant and also results in a memory leak if you will call the function like (that is if you will call the function as it is required)

    m3 = sum( m1, m2 );
    

    And this record in main

    Matrix* sum(Matrix* m1, Matrix* m2);
    

    is a function declaration. It is not a function call.

    You should write as shown above

    Matrix *m3 = NULL;
    
    //...
    
    m3 = sum( m1, m2 );