Trying to understand the problems in my code

2

I'm trying to do a function that does the following:

• Receives 3 integers from the user: size1, size2, size3.

• Creates a size1 * size2 matrix and a size2 * size3 matrix.

• Multiplies the 2 matrices.

• Prints the result matrix.

• Free all dynamic memory.

BUT after the input of the two matrix I expect the program to show the multiplication of the matrixes but it causes breakpoint in the FreeMatrix function and writes like this: Exception thrown at 0x0F82AC1D (ucrtbased.dll) in Project8.exe: 0xC0000005: Access violation reading location 0xCCCCCCC4.

The code:

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

void BuildMatrix(int*** pMat, int row, int col);
void FreeMatrix(int*** matrix, int row);
void PrintMatrix(int** pMat, int row, int col);
int** MultiplyMatrixes(int** a, int** b, int size1, int size2, int size3);

int main() {
    int** matrix1 = NULL, ** matrix2 = NULL, ** matrix3 = NULL;
    int* newCol = NULL;
    int size1, size2, size3, newRow;

    printf("-How many rows in the first matrix?: ");
    scanf("%d", &size1);
    printf("-How many columns in the first matrix and rows in the second matrix?[size2, size3]: ");
    scanf("%d", &size2);  /*size2 = rows of matrix2.*/
    printf("-How many columns in the second matrix?: ");
    scanf("%d", &size3);

    /*Build both matrices*/
    printf("-First matrix input.\n");
    BuildMatrix(&matrix1, size1, size2);
    PrintMatrix(matrix1, size1, size2);
    printf("-Second matrix input.\n");
    BuildMatrix(&matrix2, size2, size3);
    PrintMatrix(matrix2, size2, size3);

    /*Combine the 2 matrices to a new matrix*/
    matrix3 = MultiplyMatrixes(matrix1, matrix2, size1, size2, size3);
    FreeMatrix(&matrix2, size2); //Free the second matrix

    printf("\n-Multiplied matrix: \n");
    PrintMatrix(matrix3, size1, size3);

    FreeMatrix(&matrix3, size1);
    FreeMatrix(&matrix1, size1);
}

void BuildMatrix(int*** pMat, int row, int col) 
{
    int i, j;
    (*pMat) = (int**)malloc(row * sizeof(int*));
    if (*pMat == NULL) 
    {
        free(pMat);
        printf("*Not enough RAM.\nTerminating.\n");
        exit(1);
    }

    for (i = 0; i < row; i++) 
    {
        (*pMat)[i] = malloc(col * sizeof(int));
        if ((*pMat)[i] == NULL) {
            printf("*Not enough RAM.\nTerminating.\n");
            FreeMatrix(pMat, row);
            exit(1);
        }
        for (j = 0; j < col; j++) {
            printf("-Enter %d element in %d row: ", j + 1, i + 1);
            scanf("%d", &(*pMat)[i][j]);
        }
        printf("\n");
    }
    //FreeMatrix(pMat, row);
}

void FreeMatrix(int*** matrix, int row)
{
    for (int i = 0; i < row; i++)
    {
        free((matrix)[i]);
    }
    free(matrix);
}

void PrintMatrix(int** pMat, int row, int col) 
{
    for (int i = 0; i < row; ++i) 
    {
        for (int j = 0; j < col; ++j) 
        {
            printf("%d ", (pMat[i][j]));
        }
        printf("\n");
    }
}

int** MultiplyMatrixes(int** a, int** b, int size1, int size2, int size3)
{
    int i, j, k, ** c = NULL;
    c = (int**)malloc(size1 * sizeof(int*));
    if (c == NULL) 
    {
        free(*c);
        printf("*Not enough RAM.\nTerminating.\n");
        exit(1);
    }

    for (i = 0; i < size1; i++) {
        c[i] = malloc(size3 * sizeof(int));
        if (c[i] == NULL) 
        {
            printf("*Not enough RAM.\nTerminating.\n");
            FreeMatrix(&c, size1);
            exit(1);
        }

        for (j = 0; j < size3; j++) 
        {
            c[i][j] = 0;
            for (k = 0; k < size2; k++) 
            {
                c[i][j] += (a[i][k] * b[k][j]);
            }
        }
    }
}
c
visual-studio
matrix
compiler-errors
breakpoints
asked on Stack Overflow May 23, 2020 by Shay Fletcher

3 Answers

0

As when you allocate for the matrix in BuildMatrix:

(*pMat) = (int**)malloc(row * sizeof(int*));

for (i = 0; i < row; i++) 
{
    (*pMat)[i] = malloc(col * sizeof(int));
    ...
}

So, in the FreeMatrix, your code should change to:

void FreeMatrix(int*** matrix, int row)
{
    for (int i = 0; i < row; i++)
    {
        free((*matrix)[i]); // using *matrix instead of matrix
    }
    free(*matrix); // using *matrix instead of matrix also
}

You forget also to return value of MultiplyMatrixes function. It should return matrix c at the end of the function.

You do not need to free NULL pointer because if pointer is NULL, no operation is performed, for example, in your code:

   (*pMat) = (int**)malloc(row * sizeof(int*));
    if (*pMat == NULL) 
    {
        free(pMat); // it's not necessary 
        printf("*Not enough RAM.\nTerminating.\n");
        exit(1);
    }
answered on Stack Overflow May 23, 2020 by Hitokiri • edited May 23, 2020 by Hitokiri
0

There are two problems in this code. First problem, that causes your exception in FreeMatrix function: you passing int***, but you don't dereferencing first pointer.

void FreeMatrix(int*** matrix, int row)
{
    for (int i = 0; i < row; i++)
    {
        free((matrix)[i]); // Type inside free is int**, not int*
    }
    free(matrix); // Type inside free is int***, not int**
}

It should be:

void FreeMatrix(int*** matrix, int row)
{
    for (int i = 0; i < row; i++)
    {
        free((*matrix)[i]); // Type inside free is int*
    }
    free(*matrix); // Type inside free is int**
}

Also after I fixed this, I have a problem with matrix3 value in your code. You missing return statement in the MultiplyMatrixes function and matrix3 variable left unassigned.

It should be:

int** MultiplyMatrixes(int** a, int** b, int size1, int size2, int size3)
{
    int i, j, k, ** c = NULL;
    c = (int**)malloc(size1 * sizeof(int*));
    if (c == NULL)
    {
        free(*c);
        printf("*Not enough RAM.\nTerminating.\n");
        exit(1);
    }

    for (i = 0; i < size1; i++) {
        c[i] = malloc(size3 * sizeof(int));
        if (c[i] == NULL)
        {
            printf("*Not enough RAM.\nTerminating.\n");
            FreeMatrix(&c, size1);
            exit(1);
        }

        for (j = 0; j < size3; j++)
        {
            c[i][j] = 0;
            for (k = 0; k < size2; k++)
            {
                c[i][j] += (a[i][k] * b[k][j]);
            }
        }
    }

    return c; // <====
}

Also I suggest you to allocate matrices as row of pointers and a continuous array. Like this:

int** matrix = (int*)malloc(sizeof(int*) * rows); // pointers to rows
matrix[0] = (int*)malloc(rows * cols); // single chunk of memory for all elems
for(int i = 0; i < rows; ++i)
    matrix[i] = matrix[0] + i * cols;

Element access:

elem = matrix[row_index][col_index];

Freeing:

if (rows)
   free(matrix[0]);
free(matrix);

Another option is allocate matrix as a continuous array.

Like this:

int** matrix = (int*)malloc(sizeof(int*) * cols * rows); // single chunk of memory for all elems

Element access:

elem = matrix[row_index * row_size + col_index];

In this approach deallocation even more simple:

if(matrix)
   free(matrix);

Allocation as a simple chunk of memory simplifies memory deallocation and such code is much more cache friendly.

answered on Stack Overflow May 23, 2020 by Dmitrii Zabotlin • edited May 23, 2020 by Dmitrii Zabotlin
0

Here is the working code modified.
Basically the multiplication function wasn't returning the allocated address
and there was a mismatch with the triple pointer pMat.
Additionally, you should be more careful when handling bad allocated memory.
Just compare side by side your code with mine.
I tried to add useful comments.

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

void BuildMatrix(int*** pMat, int row, int col);
void FreeMatrix(int*** matrix, int row);
void PrintMatrix(int** pMat, int row, int col);
int** MultiplyMatrixes(int** a, int** b, int size1, int size2, int size3);

int main() {
    int** matrix1 = NULL, ** matrix2 = NULL, ** matrix3 = NULL;
    int* newCol = NULL;
    int size1, size2, size3, newRow;

    printf("-How many rows in the first matrix?: ");
    scanf("%d", &size1);
    printf("-How many columns in the first matrix and rows in the second matrix?[size2, size3]: ");
    scanf("%d", &size2);  /*size2 = rows of matrix2.*/
    printf("-How many columns in the second matrix?: ");
    scanf("%d", &size3);

    /*Build both matrices*/
    printf("-First matrix input.\n");
    BuildMatrix(&matrix1, size1, size2);
    PrintMatrix(matrix1, size1, size2);
    printf("-Second matrix input.\n");
    BuildMatrix(&matrix2, size2, size3);
    PrintMatrix(matrix2, size2, size3);

    /*Combine the 2 matrices to a new matrix*/
    matrix3 = MultiplyMatrixes(matrix1, matrix2, size1, size2, size3);
    FreeMatrix(&matrix2, size2); //Free the second matrix

    printf("\n-Multiplied matrix: \n");
    PrintMatrix(matrix3, size1, size3);

    FreeMatrix(&matrix3, size1);
    FreeMatrix(&matrix1, size1);
}

void BuildMatrix(int*** pMat, int row, int col) 
{
    int i, j;
    (*pMat) = (int**)malloc(row * sizeof(int*));
    if (*pMat == NULL) 
    {
        // pMat is a pointer to the "whole" matrix and mirrors
        // the calling parameter matrix1, e.g. So always use it dereferenced.
        // If *pMat is NULL there is nothing to free (at NULL).
        //free(*pMat);
        printf("*Not enough RAM.\nTerminating.\n");
        exit(1);
    }

    for (i = 0; i < row; i++) 
    {
        (*pMat)[i] = malloc(col * sizeof(int));
        //if(i == 1) //to try exception handling code
        if ((*pMat)[i] == NULL) 
        {
            int d;
            printf("*Not enough RAM.\nTerminating.\n");
            //FreeMatrix(pMat, row);
        // Your new matrix isn't complete so you should free *pMat here, 
        // and free (*pMat)[?] with ? from 0 to i-1 
            for (d = 0; d < i; ++d) free((*pMat)[d]); free(*pMat);
            exit(1);
        }
        for (j = 0; j < col; j++) {
            printf("-Enter %d element in %d row: ", j + 1, i + 1);
            scanf("%d", &(*pMat)[i][j]);
        }
        printf("\n");
    }
    //FreeMatrix(pMat, row);
}

void FreeMatrix(int*** matrix, int row)
{
    for (int i = 0; i < row; i++)
    {
        // pMat is a pointer to the "whole" matrix and mirrors
        // the calling parameter matrix1, e.g. So always use it dereferenced.
        free((*matrix)[i]);
    }
    free(*matrix);
}

void PrintMatrix(int** pMat, int row, int col) 
{
    for (int i = 0; i < row; ++i) 
    {
        for (int j = 0; j < col; ++j) 
        {
            printf("%d ", (pMat[i][j]));
        }
        printf("\n");
    }
}

int** MultiplyMatrixes(int** a, int** b, int size1, int size2, int size3)
{
    int i, j, k, ** c = NULL;
    c = (int**)malloc(size1 * sizeof(int*));
    if (c == NULL) 
    {
        // If c is NULL there is nothing to free (at NULL).
        //free(c);
        printf("*Not enough RAM.\nTerminating.\n");
        exit(1);
    }

    for (i = 0; i < size1; i++) {
        c[i] = malloc(size3 * sizeof(int));
        //if(i == 1) //to try exception handling code
        if (c[i] == NULL) 
        {
            int d;
            printf("*Not enough RAM.\nTerminating.\n");
            //FreeMatrix(&c, size1);
        // Your new matrix isn't complete so you should free c here, 
        // and free c[?] with ? from 0 to i-1   
            for (d =0; d < i; ++d) free(c[d]); free(c);
            exit(1);
        }

        for (j = 0; j < size3; j++) 
        {
            c[i][j] = 0;
            for (k = 0; k < size2; k++) 
            {
                c[i][j] += (a[i][k] * b[k][j]);
            }
        }
    }
    return c;
}
answered on Stack Overflow May 23, 2020 by quinzio • edited May 23, 2020 by quinzio

User contributions licensed under CC BY-SA 3.0