Dynamically allocate 2D array

[david73]

Hello, I'm trying to dynamically allocate 2D array. I have this function:

int **read_matrix(FILE *in, int *count) {
    int **matrix = NULL;
    char *line;
    int i = 1;
    while (!feof(in)) {

        line = readLine(in);

        matrix = (int **)realloc(matrix, (size_t)i*sizeof(int *));
        char *pch;
        pch = strtok(line, " ");
        size_t j = 1;
        while (pch != NULL) {
            matrix[i - 1] = (int *)realloc(matrix[i - 1], j * sizeof(int));
            matrix[i - 1][j - 1] = atoi(pch);
            pch = strtok(NULL, " ");
            ++j;
        }
        ++i;
    }
    *count = i;
    return matrix;
}

where "in" is open file for reading and "readline()" reads a line from file. This code is sometimes working and sometimes it is not. Can someone explain to me why? When debugging it, the problem was on this line: matrix[i - 1] = (int *)realloc(matrix[i - 1], j * sizeof(int)); but then it started working out of nowhere and I can' debug it anymore.
Thanks for your answers.

[jonnin]

do you WANT this in C or C++?

realloc is unusual here, why are you not using malloc?
I wonder if this is part of your issue?

[david73]

In C.
I need to relloc it every time because i would lost what is inserted alredy (in case of using malloc).

[jonnin]

ok. what does it DO when it 'doesnt work' ?

realloc all the time is very inefficient. It may not matter if matrix is small, but if it gets big, it will be very slow.

[david73]

Nothing really when not debugging. When debugging, it stops at that line. I will do it another though, thanks for your help. I was just wondering why this happens.

[dutch]

In your second realloc, you are assuming that matrix[i-1] will be NULL for the first call on that row. You should set it to NULL. (If the input pointer is an arbitrary value then realloc will fail and return NULL. The input pointer needs to be NULL the first time so that the first realloc call will act like a malloc.)

You aren't checking for eof correctly. It's the readLine function that will detect eof. The way you have it, when it detects eof you still continue with your loop body. Presumably readline returns NULL in case of eof.

And you aren't testing the return value of realloc for failure (NULL).

BTW, how will you know how many columns each row has? One method is to store a sentinel at the end of each row. 0, -1, or INT_MIN are natural choices, depending on your data.

Untested code. (I'm assuming you are writing this in C.)

#define END_OF_ROW INT_MIN   // INT_MIN is in <limits.h>

void *xrealloc(void *p, size_t sz) {
    p = realloc(p, sz); // not bothering to save old value since
    if (!p) {           // we are just exiting anyway
        perror("xrealloc");
        exit(EXIT_FAILURE);
    }
    return p;
}

int **read_matrix(FILE *in, int *count) {
    int **matrix = NULL;
    char *line;
    int i = 1;
    while ((line = readLine(in))) {
        matrix = xrealloc(matrix, (size_t)i*sizeof(int *));
        matrix[i - 1] = NULL;
        char *pch = strtok(line, " ");
        size_t j = 1;
        while (pch != NULL) {
            matrix[i - 1] = xrealloc(matrix[i - 1], j * sizeof(int));
            matrix[i - 1][j - 1] = atoi(pch);
            pch = strtok(NULL, " ");
            ++j;
        }
        matrix[i - 1] = xrealloc(matrix[i - 1], j * sizeof(int));
        matrix[i - 1][j - 1] = END_OF_ROW;
        ++i;
    }
    *count = i;
    return matrix;
}

It should be mentioned that this is a very inefficient way of extending an array! Normally it would be done by doubling (or maybe 150%) the current storage. This requires storing the current "capacity" along with the current "size" (elements that are in use).

If each row is supposed to have the same number of columns, then you can return the number of columns.

int **read_matrix(FILE *in, int *rows, int *cols) {
    int **matrix = NULL;
    char *line;
    int row = 0;
    *cols = 0;
    while ((line = readLine(in))) {
        ++row;
        matrix = xrealloc(matrix, (size_t)row * sizeof *matrix);
        matrix[row - 1] = NULL;
        char *pch = strtok(line, " ");
        int col = 0;
        while (pch != NULL) {
            ++col;
            matrix[row - 1] = xrealloc(matrix[row - 1], col * sizeof **matrix);
            matrix[row - 1][col - 1] = atoi(pch);
            pch = strtok(NULL, " ");
        }
        if (!*cols)
            *cols = col;
        else if (*cols != col) {
            fprintf(stderr, "Error: Bad cols\n");
            exit(EXIT_FAILURE);
        }
    }
    *rows = row;
    return matrix;
}