Dynamic matrix 2D array malloc() from external function allocation

Question

I'm trying to load file from non-main function with pointer to pointer (2D matrix) as argument in ANSI C.

Approach with l-value function is correct:

float **loadArray(int *rows, int *columns) {
    float **array;
    FILE *instream; // input file pointer
    char infile[21] = {}; //20 chars max filename length
    int i = 0, j = 0; //iterators
    printf("filename to load data from: ");
    scanf(" %20s", infile);
    if (!(instream = fopen(infile, "r"))) {
        perror("fopen() error");
        exit(-1);
    }
    fread(rows, sizeof(int), 1, instream);
    fread(columns, sizeof(int), 1, instream);
    fprintf(stdout, "\narray(%d,%d):", *rows, *columns); //OK
    // allocation of vertical array containing rows pointers.
    if (!(array = (float**)malloc((*rows) * sizeof(float*)))) {
        printf("vertical malloc() error");
        exit(-1);
    }
    for (i = 0; i < (*rows); i++) 
        // for every row allocate columns space
        if (!(array[i] = (float*)malloc((*columns) * sizeof(float)))) {
            printf("horizontal malloc() error");
            exit(-1);
        }
    for (i = 0; i < (*rows); i++)
        for (j = 0; j < (*columns); j++)
            fread((&array[i][j]), sizeof(float), 1, instream);
    fclose(instream);
    return array;
}

int main() {
    int i = 0;
    int rows = 0, columns = 0;
    float **myarray;
    myarray = loadArray(&rows, &columns);
    ...
    for (i = 0; i < rows; i++)
        free(myarray[i]);
    free(myarray);
}

But I'm trying to be consistent when reading from file and rows, columns are passed as addresses to pass the array in the same way:

int loadArray2(float ***array, int *rows, int *columns) {
    FILE *instream; // input file pointer
    char infile[21] = {}; //20 chars max filename length
    int i = 0, j = 0; //iterators
    printf("filename to load data from: ");
    scanf(" %20s", infile);
    if (!(instream = fopen(infile, "r"))) {
        perror("fopen() error");
        exit(-1);
    }
    fread(rows, sizeof(int), 1, instream);
    fread(columns, sizeof(int), 1, instream);
    fprintf(stdout,"\narray(%d,%d):", *rows, *columns); //OK
    // allocation of vertical array containing rows pointers.
    if (!(*array = (float**)malloc((*rows) * sizeof(float*)))) {
        printf("vertical malloc() error");
        exit(-1);
    }
    for (i = 0; i < (*rows); i++) 
        // for every row allocate columns space
        if (!(*array[i] = (float*)malloc((*columns) * sizeof(float)))) {
            printf("horizontal malloc() error");
            exit(-1);
        }
    for (i = 0; i < (*rows); i++)
        for (j = 0; j < (*columns); j++)
            fread((array[i][j]), sizeof(float), 1, instream);
    fclose(instream);
    return 0;
}

int main() {
    int rows = 0, columns = 0;
    float **myarray;
    loadArray2(&myarray, &rows, &columns);
    ...
    for (i = 0; i < rows; i++)
        free(myarray[i]);
    free(myarray);
}

But this approach failed. I guess I made mistake in malloc() calling either no error or warnings, or my logic is bad, I admit I'm lost...

Thanks for some tips.

Answer 1

When you use a triple pointer in loadArray2, remember that postfix operators bind more tightly than prefix operators. So you should write (*array)[i] when storing the pointer to the allocated array of float and &(*array)[i][j] when reading the float value.

Note that there are other problems in your code:

• char infile[21] = {}; is not a valid syntax in C (it is in C++ and might be supported by your compiler as an extension or maybe because you are compiling this code as C++). Write char infile[21] = ""; instead.
• Since you are reading binary data from the file, you should open it in binary mode: fopen(infile, "rb"). It might not make a difference on linux and Mac/OS, but is definitely an error on legacy systems.
• You should check for read errors.

• You could read the matrix lines in one call (below the code for the first option):

  for (i = 0; i < (*rows); i++) {
      if (fread(array[i], sizeof(float), *cols, instream) != *cols) {
          // handle the read error
      }
  }
  

Whether to use the first or second approach is debatable:

• in the first case, it is easy to return an error with a NULL pointer, so passing the indirect 2D array pointer by reference seems unnecessary. It is a more idiomatic way to return this pointer in C. If more information is needed regarding the error, you could take a pointer to the error code as an extra argument.
• in the second case, the error code can be returned directly for more precise diagnostics, but the triple pointer is error prone, as you just experienced. Such types are discouraged. Don't be a tree star programmer :)

An alternative approach is to encapsulate the pointer and the rows and columns attributes in a structure and pass a pointer to the destination structure.

Here is a modified version with this approach:

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

struct matrix {
    int cols, rows;
    float **data;
};

void matrix_reset(struct matrix *mat) {
    int i;
    if (mat->data) {
        for (i = 0; i < mat->rows; i++)
            free(mat->data[i]);
        free(mat->data);
        mat->data = NULL;
    }
    mat->rows = 0;
    mat->cols = 0;
}

int matrix_load(struct matrix *mat) {
    FILE *instream; // input file pointer
    char infile[257] = ""; //256 chars max filename length

    printf("filename to load data from: ");
    if (scanf(" %256s", infile) != 1) {
        perror("invalid input");
        return -1;
    }
    if (!(instream = fopen(infile, "rb"))) {
        perror("fopen() error");
        return -2;
    }
    if (fread(&mat->rows, sizeof(int), 1, instream) != 1
    ||  fread(&mat->cols, sizeof(int), 1, instream) != 1) {
        perror("fread() error");
        fclose(instream);
        return -3;
    }
    fprintf(stdout, "\narray(%d,%d):", mat->rows, mat->cols);
    // allocation of vertical array containing rows pointers.
    if (!(mat->data = calloc(mat->rows, sizeof(*mat->data)))) {
        printf("vertical malloc() error");
        fclose(instream);
        return -4;
    }
    for (int i = 0; i < mat->rows; i++) {
        // for every row allocate columns space
        if (!(mat->data[i] = calloc(mat->cols, sizeof(*mat->data[i])))) {
            printf("horizontal malloc() error");
            matrix_reset(mat);
            fclose(instream);
            return -5;
        }
    }
    for (i = 0; i < mat->rows; i++)
        if (fread(&mat->data[i], sizeof(*mat->data[i]), mat->cols, instream) != mat->cols) {
            printf("horizontal malloc() error");
            matrix_reset(mat);
            fclose(instream);
            return -6;
        }
    }
    fclose(instream);
    return 0;
}

int main() {
    struct matrix m = { 0, 0, NULL};

    if (!matrix_load(&m)) {
       ...
    }
    matrix_reset(&m);
    return 0;
}

Answer 2

In the failing function a major problem is that array is a pointer to a float **, a pointer you need to dereference to get the original float ** variable. Just like you do with the rows and columns arguments.

So instead of doing e.g. array[i][j] you need to do (*array)[i][j].