1

I have a task where I'm supposed to multiply two quadratic matrices of size n in C, using pointers as function parameters and return value. This is the given function head: int** multiply(int** a, int** b, int n). Normally, I would use three arrays (the two matrices and the result) as parameters, but since I had to do it this way, this is what I came up with:

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

int** multiply(int** a, int** b, int n) {
  int **c = malloc(sizeof(int) * n * n);
  // Rows of c
  for (int i = 0; i < n; i++) {
    // Columns of c
    for (int j = 0; j < n; j++) {
      // c[i][j] = Row of a * Column of b
      for (int k = 0; i < n; k++) {
        *(*(c + i) + j) += *(*(a + i) + k) * *(*(b + k) + j);
      }
    }
  }
  return c;
}

int main() {
  int **a = malloc(sizeof(int) * 2 * 2);
  int **b = malloc(sizeof(int) * 2 * 2);

  for (int i = 0; i < 2; i++) {
    for (int j = 0; i < 2; j++) {
      *(*(a + i) + j) = i - j;
      *(*(b + i) + j) = j - i;
    }
  }
  
  int **c = multiply(a, b, 2);

  for (int i = 0; i < 2; i++) {
    for (int j = 0; j < 2; j++) {
      printf("c[%d][%d] = %d\n", i, j, c[i][j]);
    }
  }

  free(a);
  free(b);
  free(c);

  return 0;
}

I have not worked much with pointers before, and am generally new to C, so I have no idea why this doesn't work or what I'd have to do instead. The error I'm getting when trying to run this program is segmentation fault (core dumped). I don't even know exactly what that means... :(

Can someone please help me out?

Lundin
  • 195,001
  • 40
  • 254
  • 396
RalphBear
  • 75
  • 1
  • 12
  • 2
    There are a few ways to create a 2d matrix in C. In one method you allocate a 1d block of memory and calculate the row/col by converting to an index. Another way is to create an array of pointers then allocate each row. You seem to be mixing the 2. See also: [Freaky way of allocating two-dimensional array?](https://stackoverflow.com/q/36794202) – 001 Jun 22 '21 at 12:38
  • `int **c = malloc(sizeof(int) * n * n);` is wrong and will not work. Do not use a pointer to a pointer to represent a matrix. I recommend using `int*` and doing index calculations manually, i.,e. `int* c = malloc(n*n*sizeof(int)); ...; c[i*n+j] = ....;` Do not use `*(c + i)` to index into an array, `c[i]` means exactly the same thing and is easier to read. – n. m. could be an AI Jun 22 '21 at 12:40
  • Do not "fix" your question once there are answers posted! That renders answers obsolete or nonsensical, and future readers won't understand a thing of the whole post. I'll rollback edits. – Lundin Jun 22 '21 at 13:18
  • @Lundin Okay, sorry. Could you still help me out and tell me how this would work with the new function header? – RalphBear Jun 22 '21 at 13:20
  • Depends on if you want to become a good C programmer or if you just want to please your incompetent teacher... In case it's the latter, someone posted a different answer about that. – Lundin Jun 22 '21 at 13:26

3 Answers3

4

There's lots of fundamental problems in the code. Most notably, int** is not a 2D array and cannot point at one.

  • i<2 typo in the for(int j... loop.
  • i < n in the for(int k... loop.
  • To allocate a 2D array you must do: int (*a)[2] = malloc(sizeof(int) * 2 * 2);. Or if you will malloc( sizeof(int[2][2]) ), same thing.
  • To access a 2D array you do a[i][j].
  • To pass a 2D array to a function you do void func (int n, int arr[n][n]);
  • Returning a 2D array from a function is trickier, easiest for now is just to use void* and get that working.
  • malloc doesn't initialize the allocated memory. If you want to do += on c you should use calloc instead, to set everything to zero.
  • Don't write an unreadable mess like *(*(c + i) + j). Write c[i][j].

I fixed these problems and got something that runs. You check if the algorithm is correct from there.

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

void* multiply(int n, int a[n][n], int b[n][n]) {
  int (*c)[n] = calloc(1, sizeof(int[n][n]));

  for (int i = 0; i < n; i++) {
    for (int j = 0; j < n; j++) {
      for (int k = 0; k < n; k++) {
        c[i][j] += a[i][k] * b[k][j];
      }
    }
  }
  return c;
}

int main() {
  int (*a)[2] = malloc(sizeof(int[2][2]));
  int (*b)[2] = malloc(sizeof(int[2][2]));

  for (int i = 0; i < 2; i++) {
    for (int j = 0; j < 2; j++) {
      a[i][j] = i - j;
      b[i][j] = j - i;
    }
  }
  
  int (*c)[2] = multiply(2, a, b);

  for (int i = 0; i < 2; i++) {
    for (int j = 0; j < 2; j++) {
      printf("c[%d][%d] = %d\n", i, j, c[i][j]);
    }
  }

  free(a);
  free(b);
  free(c);

  return 0;
}
Lundin
  • 195,001
  • 40
  • 254
  • 396
  • Thanks for the help. There is just one problem: The function header I gave in my question is given in the task and is what we're supposed to use, so I can't just change the function to have two arrays as parameters instead of pointers. It has to be `int* multiply(int* a, int* b, int n)` – RalphBear Jun 22 '21 at 13:11
  • 1
    @RalphBear Then give the link [Correctly allocating multi-dimensional arrays](https://stackoverflow.com/questions/42094465/correctly-allocating-multi-dimensional-arrays) to your teacher so that they may learn how to program correctly in C. It is particularly directed to incompetent teachers. The `int**` misconception is incredibly common. – Lundin Jun 22 '21 at 13:13
  • @Ludin I already fixed my mistake: It's pointers, not pointers to pointers. Just one `*` everywhere. That's what's given for the function header. `int* multiply(int* a, int* b, int n)` – RalphBear Jun 22 '21 at 13:15
  • @RalphBear That doesn't make much sense unless you calculate the indices manually too. You can no longer use `arr[i][j]` if you use `int*`. Nor can you use the pointer arithmetic present in the function. – Lundin Jun 22 '21 at 13:17
  • @Lundin That is why I came here in the first place. I was given this task which says to use pointers (see the corrected function header that I meationed a few times already) and have no idea how to make this work when I'm not allowed to just use arrays. – RalphBear Jun 22 '21 at 13:24
  • @RalphBear Since arrays "decay" into pointers to their first element whenever used as function parameters, `a` and `b` in `multiply(int n, int a[n][n], int b[n][n])` _are_ actually pointers. You could as well have declared the parameter as `int (*a)[n]` and it would have been 100% equivalent. – Lundin Jun 22 '21 at 13:28
2

From the updated requirement, the actual function prototype is int *multiply(int *a, int *b, int n); so the code should use a "flattened" matrix representation consisting of a 1-D array of length n * n.

Using a flattened representation, element (i, j) of the n * n matrix m is accessed as m[i * n + j] or equivalently using the unary * operator as *(m + i * n + j). (I think the array indexing operators are more readable.)

First, let us fix some errors in the for loop variables. In multiply:

      for (int k = 0; i < n; k++) {

should be:

      for (int k = 0; k < n; k++) {

In main:

    for (int j = 0; i < 2; j++) {

should be:

    for (int j = 0; j < 2; j++) {

The original code has a loop that sums the terms for each element of the resulting matrix c, but is missing the initialization of the element to 0 before the summation.

Corrected code, using the updated prototype with flattened matrix representation:

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

int* multiply(int* a, int* b, int n) {
  int *c = malloc(sizeof(int) * n * n);
  // Rows of c
  for (int i = 0; i < n; i++) {
    // Columns of c
    for (int j = 0; j < n; j++) {
      // c[i][j] = Row of a * Column of b
      c[i * n + j] = 0;
      for (int k = 0; k < n; k++) {
        c[i * n + j] += a[i * n + k] * b[k * n + j];
      }
    }
  }
  return c;
}

int main() {
  int *a = malloc(sizeof(int) * 2 * 2);
  int *b = malloc(sizeof(int) * 2 * 2);

  for (int i = 0; i < 2; i++) {
    for (int j = 0; j < 2; j++) {
      a[i * 2 + j] = i - j;
      b[i * 2 + j] = j - i;
    }
  }
  
  int *c = multiply(a, b, 2);

  for (int i = 0; i < 2; i++) {
    for (int j = 0; j < 2; j++) {
      printf("c[%d][%d] = %d\n", i, j, c[i * 2 + j]);
    }
  }

  free(a);
  free(b);
  free(c);

  return 0;
}
Ian Abbott
  • 15,083
  • 19
  • 33
1

You need to fix multiple errors here:

1/ line 5/24/28: int **c = malloc(sizeof(int*) * n )

2/ line 15: k<n

3/ Remark: use a[i][j] instead of *(*(a+i)+j)

4/ line 34: j<2

5/ check how to create a 2d matrix using pointers.

#include <stdio.h>
#include <stdlib.h>
  
int** multiply(int** a, int** b, int n) {
  int **c = malloc(sizeof(int*) * n );
  for (int i=0;i<n;i++){
    c[i]=malloc(sizeof(int) * n );
  }
  
  // Rows of c
  for (int i = 0; i < n; i++) {
    // Columns of c
    for (int j = 0; j < n; j++) {
      // c[i][j] = Row of a * Column of b
      for (int k = 0; k < n; k++) {
    c[i][j] += a[i][k] * b[k][j];
      }
    }
  }
  return c;
}

int main() {
  int **a = malloc(sizeof(int*) * 2);
  for (int i=0;i<2;i++){
    a[i]=malloc(sizeof(int)*2);
  }
  int **b = malloc(sizeof(int) * 2);
  for (int i=0;i<2;i++){
    b[i]=malloc(sizeof(int)*2);
  }

  for (int i = 0; i < 2; i++) {
    for (int j = 0; j < 2; j++) {
      a[i][j] = i - j;
      b[i][j] = i - j;
    }
  }

  int **c = multiply(a, b, 2); 

  for (int i = 0; i < 2; i++) { 
    for (int j = 0; j < 2; j++) { 
      printf("c[%d][%d] = %d\n", i, j, c[i][j]); 
    } 
  } 
  free(a);
  free(b);
  free(c);
  return 0;
}
hakim
  • 139
  • 15
  • 1
    This doesn't create 2D arrays though. It is needlessly slow and needlessly complex. See [Correctly allocating multi-dimensional arrays](https://stackoverflow.com/questions/42094465/correctly-allocating-multi-dimensional-arrays). – Lundin Jun 22 '21 at 13:02