Matrix product cause error: subscripted value is neither array nor pointer nor vector

Question

I have a Java method that multiplies 2 matrices. I have tried to port the same method in C without success.

Here is my attempt to write a method that should multiply 2 matrices in C:

float **multiply(int m1, int n1, float Xy1[], int m2, int n2, float Xy2[]) {
    int i, j, k;
    float **result = allocate_mem_mtrx(m1, n2);
    for (i = 0; i < m1; i++) {
        for (j = 0; j < n2; j++) {
            for (k = 0; k < n1; k++) {
                result[i][j] = result[i][j] + Xy1[i][k] * Xy2[k][j];
            }
        }
    }
    return result;
}

At this line

result[i][j] = result[i][j] + Xy1[i][k] * Xy2[k][j];

I receive the error:

subscripted value is neither array nor pointer nor vector

Clearly my syntax is wrong, but I haven't understood how I should fix this line of code, to solve my problem.

In my main I have:

float matrix1[3][2] = {{0, 1}, {3, 4}, {6, 7}};
float matrix2[2][3] = {{5, 1, 2}, {3, 4, 5}}; 

with the actual signature I invoke the method in this way:

multiply(3, 2, &matrix1, 2, 3, &matrix2);

---

My original Java method

public static int[][] multiply(int[][] Xy1, int[][] Xy2) {
    int rowsInXy1 = Xy1.length;
    int columnsInXy1 = Xy1[0].length; // same as rows in B
    int columnsInXy2 = Xy2.length;
    int[][] result = new int[rowsInXy1][columnsInXy2];
    for (int i = 0; i < rowsInXy1; i++) {
        for (int j = 0; j < columnsInXy2; j++) {
            for (int k = 0; k < columnsInXy1; k++) {
                result[i][j] = result[i][j] + Xy1[i][k] * Xy2[k][j];
            }
        }
    }
    return result;
}

Answer 1

You have float Xy1[]

but you treat it as 2D

Xy1[i][k]

Same for Xy2.

You should change float Xy1[] to float** Xy1.

Also another thing, in your loop I feel that you are sure that result 2D array is initialized in your function that says allocate. If that functions just mallocs the array, then this array will have garbage inside. For more.

[EDIT]

Also I see in one other answer that they cast what malloc returns. Ouch!

Do not cast the return value of malloc in C.

[EDIT.2]

SO, you could something like this:

#include <stdlib.h>

// We return the pointer
float **get(int N, int M) /* Allocate the array */
{
    /* Check if allocation succeeded. (check for NULL pointer) */
    int i;
    float **table;
    table = malloc(N*sizeof(float *));
    for(i = 0 ; i < N ; i++)
        table[i] = malloc( M*sizeof(float) );
    return table;
} 

void free2Darray(int** p, int N) {
    int i;
    for(i = 0 ; i < N ; i++)
        free(p[i]);
    free(p);
}

// do not forget to FREE what multiply returns
float **multiply(int m1, int n1, float** Xy1,int m2, int n2, float** Xy2) {
    int i,j,k;
    float **result = get(m1,n2);
    for (i = 0; i < m1; i++) {
        for (j = 0; j < n2; j++) {
            for (k = 0; k < n1; k++) {
                // this line is correct, the prototype of the function
                // was not OK
                result[i][j] = result[i][j] + Xy1[i][k] * Xy2[k][j];
            }
        }
    }
    return result;
}
int main(void) {
  float** A = get(5, 5); // arrays declared as double pointers
  float** B = get(5, 5);
  float** C = get(5, 5);

  C = multiply(5, 5, A, 5, 5, B);

  // free2Darray function defined below
  free2Darray(A, 5);
  free2Darray(B, 5);
  free2Darray(C, 5);

  return 0;
}

[EDIT.3]

Another way, if you now the columns apriori, which you probably don't as your function implies, you could do that:

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

// We return the pointer
float **get(int N, int M) /* Allocate the array */
{
    /* Check if allocation succeeded. (check for NULL pointer) */
    int i;
    float **table;
    table = malloc(N*sizeof(float *));
    for(i = 0 ; i < N ; i++)
        table[i] = malloc( M*sizeof(float) );
    return table;
}

float **multiply(int m1, int n1, float Xy1[][2],int m2, int n2, float Xy2[][2]) {
    int i,j,k;
    float **result = get(m1,n2);
    for (i = 0; i < m1; i++) {
        for (j = 0; j < n2; j++) {
            for (k = 0; k < n1; k++) {
                result[i][j] = result[i][j] + Xy1[i][k] * Xy2[k][j];
            }
        }
    }
    return result;
}
int main() {
  float matrix1[2][2] = {{0, 1}, {3, 4}};
  float matrix2[2][2] = {{0, 1}, {3, 4}};
  float** C;

  C = multiply(2, 2, matrix1, 2, 2, matrix2);

  free2Darray(C, 2);

  printf("ok\n");

  return 0;
}

I got the get() function from my pseudo-site.

Answer 2

float Xy1[]

is not a two dimensional array, so to write

Xy1[i][k]

is an error.

In C you can use a pointer to pointer notation:

float** multiply( int m1, int n1, float** Xy1,int m2, int n2, float** Xy2) {
    //...
}

or you can take advantage of the variable-length array feature in the C language and write functions that can take multidimensional arrays of varying sizes:

float** multiply( int m1, int n1, float Xy1[m1][n1],
                                       int m2, int n2, float Xy2[m2][n2]) {
        float **result, i, j;

        result = malloc( m1 * sizeof *result);  // remember to free this memory

        for( i = 0; i < m1; ++i)
            result[i] =  malloc( n2 * sizeof float);

        //... I don't continue since there seems to be an issue with your indices
        // however now you can use result this way:
        // result[i][j] = result[i][j] + Xy1[i][k] * Xy2[k][j]; i, j, k integers
        return result;
}

In C++ use a std::vector< std::vector<float> >:

typedef std::vector<std::vector<float> > array;  // shorten notation

array multiply( const array& Xy1, const array& Xy2) {
    //...
}

In addition

int[][] result = new int[rowsInXy1][columnsInXy2];

is not correct way to create two dimensional array. Such array is an array of pointers and correct way to create this is:

C

int **a, i;

a = malloc( rowsInXy1 * sizeof *a);

for( i = 0; i < rowsInXy1; ++i)
    a[i] =  malloc( columnsInXy2 * sizeof int);

C++

int** result = new int*[rowsInXy1];
for( int i = 0; i < rowsInXy1; ++i)
    result[i] = new int[columnsInXy2];

Answer 3

I see you have already chosen a solution but here is some additional info about arrays in C.

In C there is no equivalent of Java's int[][] type. Java array storage also includes storage of how many items are in the array. However, C's array storage is purely the values in the array contiguous in memory; and you have to keep track of the size either as compile-time constants, or by storing a variable that holds the length.

Further; originally in C, arrays (using the [] syntax) had to have their size known at compile-time. There is an optional feature called variable-length array which means that the array size can be specified at runtime -- however the size can't be changed once the array is created. The major compiler vendors all support VLA, I think.

The VLA uses contiguous storage, however it is not possible to return one via a function's return value. But you can "return" them using an "out" parameter.

It's probably going to turn into a jumble if your code sometimes uses VLAs and sometimes uses dynamically-allocated arrays of pointers.

Another point: in both cases you have to store both array dimensions separately from the array, so that's 3 variables to pass around for each matrix. I'd like to suggest wrapping all of this up in a struct. Since VLAs can't live in a struct, and the compiler might not support them anyway, your best bet may be to use the dynamic allocation version.

Even though we are in C, if you are planning to do more than just a throwaway program, my recommendation would be to use an object-oriented approach for the matrix. Here's an example framework (where all Matrices are to have the Matrix be dynamically allocated, as well as the cells):

// Matrix.h
typedef struct
{
    size_t y_dim, x_dim;
    float **cells;
} Matrix;

Matrix *matrix_construct(size_t y_dim, size_t x_dim);
void matrix_free(Matrix *m);     // frees m as well as cells

void matrix_assign(Matrix *dst, Matrix const *src);   // may re-size dst
void matrix_set_values(Matrix *dst, float *row_major);
bool matrix_is_equal(Matrix const *m1, Matrix const *m2);   

typedef Matrix *MatrixBinaryOperator(Matrix const *m1, Matrix const *m2);
MatrixBinaryOperator matrix_multiply;

Example usage:

float mat1_array[3][2] = {{0, 1}, {3, 4}, {6, 7}};
float mat2_array[2][3] = {{5, 1, 2}, {3, 4, 5}}; 

Matrix *mat1 = matrix_construct(3, 2);
Matrix *mat2 = matrix_construct(2, 3);
matrix_set_values(mat1, (float *)&mat1_array);
matrix_set_values(mat2, (float *)&mat2_array);

Matrix *mat3 = matrix_multiply(mat1, mat2);

matrix_free(mat1);    
matrix_free(mat2);    
matrix_free(mat3);    

See it working - disclaimer: this code doesn't check for malloc failure and I have only done the most basic testing; and a lot of optimization is possible.