-1

I am trying to implement matrix multiplication in C. I am getting segmentation fault error if i use matrix size more than 3. For 2x2 matrix this code it working perfectly. I'm trying to figure out the reason for this.

Here is my code. Please have a look at it and let me know where I'm doing wrong. 

#include <stdio.h>
#include <stdlib.h>     
#include <time.h>
/*  
matrix data structure. 
rs = row start
re = row end
cs = column start 
ce = column end
a = pointer to array of pointers
*/       

typedef struct _matrix {
    int rs;
    int re;
    int cs;
    int ce;
    int **a ;
}matrix;

matrix random_matrix(int n)
{
    matrix random;
    int i, j, k; 

    random.a = (int **)malloc(sizeof(int *) * n);
    for (k=0; k < n; k++)
         random.a[k] = (int *)malloc(n * sizeof(int));

    random.cs = random.rs = 0;
    random.ce = random.re = n -1; 

    for(i=0; i < n; i++){
        for(j = 0; j < n; j++){
            random.a[i][j] = rand()/108108108.0;
    }
}

    return random;       
}

void display(matrix m)
{
    int i, j;

    for (i=m.rs ; i<=m.re ; i++) {
        for (j=m.cs ; j<=m.ce ; j++) {
            if(j==m.ce)
                printf("%d", m.a[i][j]);
            else 
                printf("%d, ", m.a[i][j]);
        }
    printf("\n");
    }
    printf("\n");

    return;
}

matrix multiply(matrix m1, matrix m2)
{
    int n = m1.re - m1.rs;
    matrix result;

    result.rs = result.cs = 0;
    result.re = result.ce = n;

    result.a = (int **)malloc(sizeof(int *) * n);
    for (int k=0; k < n; k++)
         result.a[k] = (int *)malloc(n * sizeof(int));

    for (int i = 0; i < n; i++) {
        for (int j = 0; j < n; j++) {
            float sum = 0;
            for (int k = 0; k < n; k++)
                sum += m1.a[i][k] * m2.a[k][j];
            result.a[i][j] = sum;
        }
    }

    return result;
}

int main(void)
{ 
    srand(time(NULL));

    matrix m1 = random_matrix(3);
    matrix m2 = random_matrix(3);

    display(m1);
    display(m2);

    printf("   RESULT    \n");
    display(multiply(m1, m1));   

    return 0;
}
emperorspride188
  • 819
  • 1
  • 8
  • 13
  • 2
    First run in a debugger to catch the crash in action, so you can learn where in your code it happens. Then you should know that in C [you don't need (and should not) cast the result of `malloc`](http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc) (or any function returning `void *`). – Some programmer dude Feb 07 '16 at 19:38
  • Use meaningful names for identifiers, I am having a hard time understanding your code because of that. – Iharob Al Asimi Feb 07 '16 at 19:40
  • 1
    Why don't you debug the program yourself rather than just dump it on SO for someone else to do it for you? You'll learn more in the long run if you persevere and try to do it yourself first. – kaylum Feb 07 '16 at 19:41

1 Answers1

0

Your very confusing rs, re, cs, ce that lead to "unnatural"1 loops starting at rs or cs and ending at re or ce are the reason, in multiply you have

result.re = result.ce = n;

but should be

result.re = result.ce = n - 1;

Never do that, just use two of this indicators called rows and columns probably, and your loops

for (int row = 0 ; row < m.rows ; ++row)
{
    for (int column = 0 ; column < m.columns ; ++column)
    {
    }
}

and avoid the confusion completely.

Note: There is no free() in your code, for every malloc() there has to be a free(), it's not a huge problem with your code but consider a long lived program like a daemon/service (depends on how you call them, but they are the same), there a small memory leak would become a major issue.

void free_matrix(matrix *m)
{
    for (int i = m->rs ; i <= m->re ; ++i)
        free(m->a[i]);
    free(m->a);
}

1Unnatural in the sense, that it's not easy for an experienced programmer to read them correctly, and can cause the very bug in your code you are dealing with now.

Iharob Al Asimi
  • 52,653
  • 6
  • 59
  • 97