My code stoped working after i changed the notation

Question

I am very new at pointers and arrays. I would appreciate any help.

I'm trying to write a matrix multiplication code. At first my code looked like this:

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

void populate (int inp_row,int inp_col,int *arr);
void multiplication(int *arr1,int *arr2,int *arr3,int inp_row,int inp_col);
void display(int *arr,int inp_row,int inp_col);

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

    int row,col;

    printf("Enter number of rows: ");
    scanf("%d",&row);

    printf("Enter number of columns: ");
    scanf("%d",&col);

    int*arrA=(int *)malloc(row*col*sizeof(int));
    int*arrB=(int *)malloc(row*col*sizeof(int));
    int*arrC=(int *)malloc(row*col*sizeof(int));

    populate(row,col,arrA);
    populate(row,col,arrB);

    multiplication(arrA,arrB,arrC,row,col);

    printf("Your Matrices are as follows:\n=============================");
    printf("\nMatrix A is as follows:\n");
    display(arrA,row,col);
    printf("\nMatrix B is as follows:\n");
    display(arrB,row,col);
    printf("\nResults:\n=============================");
    printf("\nA * B is as follows:\n");
    display(arrC,row,col);
}

void populate (int inp_row,int inp_col,int *arr){
    int i,j;

    for(i=0;i<inp_row;i++)
        for(j=0;j<inp_col;j++)
            *(arr + inp_row*i + j)=rand()%11;
}

void multiplication(int *arr1,int *arr2,int *arr3,int inp_row,int inp_col){
    int i,j,k;

    for(i=0;i<inp_row;i++)
        for(j=0;j<inp_col;j++)
            *(arr3 + inp_row*i + j)=0;

    for(i=0;i<inp_row;i++)
        for(j=0;j<inp_col;j++)
            for(k=0;k<inp_col;k++)
                *(arr3 + inp_row*i + j)=(*(arr1 + inp_row*i +k)*(*(arr2 + inp_row*k + j))+*(arr3 + inp_row*i + j));
}

void display(int *arr,int inp_row,int inp_col){
    int i,k;

    for(i=0;i<inp_row;i++){
        printf("\n|");printf("%d",*(arr + inp_row*i + 0));
        for(k=0;k<(inp_col-1);k++)
            printf("  %d",*(arr + inp_row*i + k));
        printf("|");
    }
}

Printing seemed fine, it didn't stop working. The only issue looked like it was the math. The multiplication was giving the wrong answers. I looked at some online resources and i thought my algorithm looked ok. Then i basically started screwing around with the code..

I looked through my lecture notes and changed the array notations and added memmory allocation error messages.

Now the code stops working after asking for the inputs and i can't seem to understand.

I am also new to this notation of dynamic arrays. Normally i wrote them like my previous code. That might be the issue.

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

void populate (int inp_row,int inp_col,int **arr);
void multiplication(int **arr1,int **arr2,int **arr3,int inp_row,int inp_col);
void display(int **arr,int inp_row,int inp_col);

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

    int row,col;

    printf("Enter number of rows: ");
    scanf("%d",&row);

    printf("Enter number of columns: ");
    scanf("%d",&col);

    int**arrA=(int **)malloc(row*sizeof(int));
    int**arrB=(int **)malloc(row*sizeof(int));
    int**arrC=(int **)malloc(row*sizeof(int));

    if(arrA == NULL || arrB == NULL|| arrC == NULL){
        printf("\nCouldn't allocate the memmory!");
        return -1;}

    int i;
    for(i=0;i<col;i++){
        arrA[i]=(int*)malloc(sizeof(int)*col);
        arrB[i]=(int*)malloc(sizeof(int)*col);
        arrC[i]=(int*)malloc(sizeof(int)*col);
        if(arrA[i] == NULL || arrB[i] == NULL|| arrC[i] == NULL){
            printf("\nCouldn't allocate the memmory!");
            return -2;}}

    populate(row,col,arrA);
    populate(row,col,arrB);

    multiplication(arrA,arrB,arrC,row,col);

    printf("Your Matrices are as follows:\n=============================");
    printf("\nMatrix A is as follows:\n");
    display(arrA,row,col);
    printf("\nMatrix B is as follows:\n");
    display(arrB,row,col);
    printf("\nResults:\n=============================");
    printf("\nA * B is as follows:\n");
    display(arrC,row,col);
}

void populate (int inp_row,int inp_col,int **arr){
    int i,j;

    for(i=0;i<inp_row;i++)
        for(j=0;j<inp_col;j++)
            arr[i][j]=rand()%11;
}

void multiplication(int **arr1,int **arr2,int **arr3,int inp_row,int inp_col){
    int i,j,k,sum=0;

    for(i=0;i<inp_row;i++)
        for(j=0;j<inp_col;j++)
            arr3[i][j]=0;

    for(i=0;i<inp_row;i++)
        for(j=0;j<inp_col;j++)
            for(k=0;k<inp_col;k++){
                sum+=arr1[i][k]*arr2[k][j];}
    arr3[i][j]=sum;
}

void display(int **arr,int inp_row,int inp_col){
    int i,k;

    for(i=0;i<inp_row;i++){
        printf("\n|");printf("%d",arr[i][0]);
        for(k=0;k<(inp_col-1);k++)
            printf("  %d",arr[i][k]);
        printf("|");
    }
}

The two codes are quite different, why do you think they should behave the same? Also show a simple example of input that triggers the error. — Jabberwocky, Apr 24 '20 at 14:35
You iterate over 0..col, but the malloc for arr* is of size row. There are probably other errors. Use valgrind, and a debugger. — PiRocks, Apr 24 '20 at 14:36

Craig Estey · Answer 1 · 2020-04-24T15:46:01.753

If you run under gdb, your program is segfaulting in multiplication in the first loop.

That is because your first batch of malloc calls in main is incorrect.

Change:

int **arrA = (int **) malloc(row * sizeof(int));
int **arrB = (int **) malloc(row * sizeof(int));
int **arrC = (int **) malloc(row * sizeof(int));

Into:

int **arrA = (int **) malloc(row * sizeof(int *));
int **arrB = (int **) malloc(row * sizeof(int *));
int **arrC = (int **) malloc(row * sizeof(int *));

An int is 32 bits, but [obviously] you're on a 64 bit machine, so a pointer is 64 bits. So, you're not allocating enough space for the pointers.

This is undefined behavior, which can take some time to cause a fault. My guess is that the populate call trashed memory, but didn't kill itself. But, what it left behind were invalid pointer values that multiplication tried to dereference.

If you compile with -Wall -O2, the following line in multiplication is flagged:

arr3[i][j] = sum;

With:

warning: ‘j’ may be used uninitialized in this function [-Wmaybe-uninitialized]

As MrBens mentioned, this line should probably go inside the loop

But, the math still doesn't look right to me. I'm a bit rusty on matrix multiplication, but I looked it up on wikipedia [and I'm pre-coffee, so ...]

I think you should zero out sum inside the loop.

So, I'd change this:

for (i = 0; i < inp_row; i++) {
    for (j = 0; j < inp_col; j++) {
        for (k = 0; k < inp_col; k++) {
            sum += arr1[i][k] * arr2[k][j];
        }
    }
}
arr3[i][j] = sum;

Into:

for (i = 0; i < inp_row; i++) {
    for (j = 0; j < inp_col; j++) {
        sum = 0;
        for (k = 0; k < inp_col; k++)
            sum += arr1[i][k] * arr2[k][j];
        arr3[i][j] = sum;
    }
}

MrBens · Answer 2 · 2020-04-24T16:10:29.913

I think your mistake is in this section:

for(i=0;i<inp_row;i++)
    for(j=0;j<inp_col;j++)
        for(k=0;k<inp_col;k++){
            sum+=arr1[i][k]*arr2[k][j];}
        arr3[i][j]=sum;}

The last line is not included in the calculation of every row! You should change it to this:

for(i=0;i<inp_row;i++)
{
    for(j=0;j<inp_col;j++)
    {
        for(k=0;k<inp_col;k++)
        {
            sum+=arr1[i][k]*arr2[k][j];
        }
        arr3[i][j]=sum;
    }
}

Regarding the rest of the code, you have some language-related errors, as PiRocks mentioned. The most shouting one is that you don't free the resources you use! This is a bad practice and just plain wrong.

Add this to the end of your main function, and be sure to understand the meaning of it:

for(i = 0; i < col; i++)
{
    free(arrA[i]);
    free(arrB[i]);
    free(arrC[i]);
}

free(arrA);
free(arrB);
free(arrC);

You should definitely get the habit of using valgrind when dealing with dynamic memory allocations.

Edit: Craig is right by that you should init sum to 0 every iteration, missed that.

Perhaps you should reference: https://stackoverflow.com/questions/359732/why-is-it-considered-a-bad-practice-to-omit-curly-braces?lq=1 — Barmar, Apr 24 '20 at 14:58

My code stoped working after i changed the notation

2 Answers2