1

I have an assignment where i have to make a matrix multiplication program more efficient so i wrote a method called multiply matrix but after i actually do the matrix multiplication in a loop the final product matrix is all zero but, if i check while in the loop its not zero

int** multiply_matrices(int** matrix1, int** matrix2, int m1, int n1, int m2, int n2) {
if(n1 != m2) {
    printf("error\n");
    return NULL;
}

int i = 0;
int j = 0;
int k = 0;
int** product = (int**) malloc(m1 * sizeof(int*));



for(i = 0; i<m1; i++){
    product[i] = (int*) malloc(n2 * sizeof(int));
    for(j = 0; j<n1; j++){
        product[i][j] = 0;
    }
}
*
for(i = 0; i < m1; i ++) {
    product[i] = (int*) malloc(n2 * sizeof(int));
    int chg = 0;

     while(j<n2){


        if(k == n1 ){
            //chg = 0;
            k = 0;
            //printf("%d\n", j);
            j++;
        }

             product[i][j]  += matrix1[i][k] * matrix2[k][j];

            printf("%d \n", product[i][j]);
        k++;
        chg = 1;

    }
}

return product;

}

JAY PATEL
  • 25
  • 2
  • 1
    You are reserving memory twice? for `product[i]`, you will deplete your memory resources with these memory leak because the caller can't free it. What is the _asterisk_ between the two for loops for? – TrustworthySystems Apr 26 '18 at 16:43

1 Answers1

1

Two errors I found:

You aren't resetting j = 0 in the outer multiplication loop. This leads to it only completing one row, if it enters the inner loop at all, as its value remains equal to n1 after the allocation loop. This is most likely why it is returning all zeros for you.

Second, you should reset k in the outer loop as well. When the condition j<n2 is broken, i increments but k remains the same value as when the last loop ended.

I also do not know what the point of the chg variable is as is it doing nothing.

And, as TrustworthySystems said, only allocate memory once or you will have leaks. Here is the correct function:

int** multiply_matrices(int** matrix1, int** matrix2, int m1, int n1, int m2, int n2) {
if(n1 != m2) {
    printf("error\n");
    return NULL;
}

int i = 0;
int j = 0;
int k = 0;
int** product = (int**) malloc(m1 * sizeof(int*));



for(i = 0; i<m1; i++){
    product[i] = (int*) malloc(n2 * sizeof(int));
    for(j = 0; j<n1; j++){
        product[i][j] = 0;
    }
}

for(i = 0; i < m1; i++) {
    int chg = 0;
    j = 0;
    k = 0;

    while(j<n2){
        if(k == n1 ){
            k = 0;
            j++;
        }
        product[i][j]  += matrix1[i][k] * matrix2[k][j];
        k++;
        chg = 1;

    }
}
return product;
}
Bwood94
  • 214
  • 1
  • 7
  • 1
    There is no need to cast the return of `malloc`, it is unnecessary. See: [Do I cast the result of malloc?](http://stackoverflow.com/q/605845/995714) – David C. Rankin Apr 26 '18 at 17:37