0

I want to write a program that calculates a matrix multiplication.

However, it seems like there is a problem concerning the memory allocation. For m <= 2 and n <= 2, the code works just fine but after staring at it for an hour, I still can't figure out why the program blows up vor values greater than that (SegFault and free_matrix() is complaining about trying to free not allocated memory).

Here is my Code:

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

typedef double *Vector;
typedef Vector *Matrix;

//Initializes a matrix of given size
Matrix mat_alloc(int m, int n){
    Matrix mat = NULL;
    mat = (Matrix)malloc(sizeof(m * sizeof(Vector)));
    if(mat == NULL){
        printf("Error: Not Enough Memory!\n");
        return NULL;
    }
    for(int i = 0; i < m; i++){
        mat[i] = (Vector)malloc(n * sizeof(double));
        if(mat[i] == NULL){
            printf("Error: Not Enough Memory!\n");
            return NULL;
        }
    }
    return mat;
}

Matrix mat_mult(Matrix A, Matrix B, int m, int n, int k){
    Matrix C = mat_alloc(m, k);
    for(int i = 0; i < m; i++){
        for(int j = 0; j < k; j++){
            C[i][j] = 0;
            for(int l = 0; l < n; l++){
                C[i][j] += A[i][l] * B[l][j];
            }
        }
    }

    return C;
}

void print_matrix(Matrix mat, int m, int n){
    for(int i = 0; i < m; i++){
        for(int j = 0; j < n; j++){
            printf("%.2lf ", mat[i][j]);
        }
        printf("\n");
    }
}

void read_matrix(Matrix mat, int m, int n){
    for(int i = 0; i < m; i++){
        for(int j = 0; j < n; j++){
            printf("(%d,%d) = ", i+1, j+1);
            scanf("%lf", &mat[i][j]);
        }
    }
}

void free_matrix(Matrix mat, int m){
    for(int i = 0; i < m; i++){
        free(mat[i]);
    }
    free(mat);
}

int main(int argc, char *argv[]){

    int m = 0;
    int n = 0;
    int k = 0;

    printf("Dimensions of A (m x n):\n");
    printf("m = ");
    scanf("%d", &m);
    printf("n = ");
    scanf("%d", &n);

    printf("Dimensions of B (n = %d x k):\n", n);
    printf("k = ");
    scanf("%d", &k);

    printf("Your input: m = %d, n = %d, k = %d\n", m, n, k);

    Matrix A = NULL;
    Matrix B = NULL;
    Matrix C = NULL;

    A = mat_alloc(m, n);
    B = mat_alloc(n, k);

    printf("Enter Values for A!\n");
    read_matrix(A, m, n);

    printf("Enter Values for B!\n");
    read_matrix(B, n, k);

    printf("A = \n");
    print_matrix(A, m, n);

    printf("\nB = \n");
    print_matrix(B, n, k);

    C = mat_mult(A, B, n, m, k);
    printf("\nC = \n");
    print_matrix(C, m, k);

    free_matrix(A, m);
    free_matrix(B, n);
    free_matrix(C, m);

    return 0;
}

Thank you in advance.

  • 1
    Is all this code necessary to reproduce the problem? Looks like you could skip at least `mat_mult` and hard code problematic values instead of using `scanf`. Take your time to create a [mre]. And [don't cast malloc](https://stackoverflow.com/q/605845/6699433) – klutt Feb 04 '20 at 13:43
  • 2
    Change `mat = (Matrix)malloc(sizeof(m * sizeof(Vector)));` to `mat = malloc(m * sizeof(*mat));` and save yourself some head ache – klutt Feb 04 '20 at 13:49
  • 1
    Hiding pointers behind typedefs is the root of all evil. Instead of this alien pointer (to pointer) language you have invented, you could be using dynamically allocated 2D arrays instead. https://stackoverflow.com/questions/42094465/correctly-allocating-multi-dimensional-arrays – Lundin Feb 04 '20 at 13:56
  • suggestion: use VLS's to simply the allocation. `A[m][n];` `B[n][k]` and `C[m][k]` – TruthSeeker Feb 04 '20 at 14:22
  • @TruthSeeker What is VLS? Do you mean C's _variable length array_ [VLA](https://en.wikipedia.org/wiki/Variable-length_array)? – chux - Reinstate Monica Feb 04 '20 at 15:11
  • Welcome to Stack Overflow! Please [edit] your question to show us what kind of debugging you've done. I expect you to have run your [mcve] within Valgrind or a similar checker, and to have investigated with a debugger such as GDB, for example. Ensure you've enabled a full set of compiler warnings, too. What did the tools tell you, and what information are they missing? And read Eric Lippert's [How to debug small programs](https://ericlippert.com/2014/03/05/how-to-debug-small-programs/). – Toby Speight Feb 04 '20 at 17:27
  • @chux-ReinstateMonica: it was a typo. I meant VLA's – TruthSeeker Feb 05 '20 at 02:34

1 Answers1

4

This here:

mat = (Matrix)malloc(sizeof(m * sizeof(Vector)));

Should be

mat = (Matrix)malloc(m * sizeof(Vector));

Or better yet

mat = malloc(sizeof *mat * m);

You have a sizeof too much, so instead of getting the desired size of the pointer array, you get a constant (the size of a size_t).

Blaze
  • 16,736
  • 2
  • 25
  • 44