CODEWITHSUNDEEP
cpointersmalloc
cost
cost

Reputation: 4490

Problem mallocing a 2d array

I'm still new to C, malloc, and all that jazz, so I decided to write this to learn some more skills. The idea is, I'm reading in a bunch of ints from a file and putting them into a matrix (2d array). The start of the file says how many rows and columns there are, so it reads those numbers in and uses malloc to set up the 2d array.

int read_matrix(FILE *mat, int ***Z, int *x, int *y) 
{
    int i = 0;
    int x_temp = 0;
    int y_temp = 0;

    if (fscanf(mat, "%d %d", &(*x), &(*y)) == EOF){
        printf("File is not big enough to contain a matrix\n");
        return -1;
    }

    printf("About to malloc %d\n", *x);

    *Z = (int**) malloc(*x * sizeof(int*));
    while (i < *x) {
        printf("mallocing %d\n", i);
        *Z[i] = (int*) malloc(*y * sizeof(int));
        printf("malloced\n");
        ++i;
    }

    printf("Malloc complete\n");

    /*Other unimportant code*/
}

The output reads:

About to malloc 3 
mallocing 0 
malloced 
mallocing 1
Segmentation fault

So it's not mallocing anything but one int** in Z.. I think?

I'm very new to C, so I'm not sure if I've made some little mistake, or if I'm really going about this whole thing incorrectly. Any thoughts? Thanks!

Upvotes: 2

Views: 339

Answers (3)

caf
caf

Reputation: 239371

I agree with both Walter and AndreyT. This is just some additional information.

Note that you can get away with just two malloc() calls, rather than *x + 1 - one big block for the ints themselves, and one for the row index.

*Z = malloc(*x * sizeof (*Z)[0]);
(*Z)[0] = malloc(*x * *y * sizeof (*Z)[0][0]);
for (i = 1; i < *x; i++) {
    (*Z)[i] = (*Z)[0] + i * *y;
}

Upvotes: 2

AnT stands with Russia
AnT stands with Russia

Reputation: 320797

As Walter correctly noted in his answer, it should be (*Z)[i] = ..., not *Z[i] = ....

On top of that I'd suggest getting rid of that dereference/typecast hell present in your source code. Don't cast the result of malloc. Don't use type names under sizeof. Expressing it as follows

 *Z = malloc(*x * sizeof **Z);
 ...
 (*Z)[i] = malloc(*y * sizeof *(*Z)[i]);

wil make your code type-independent and much more readable.

A separate question is what on Earth made you use &(*x) in fscanf. Is this some kind of bizarre coding standard?

Upvotes: 1

Walter Mundt
Walter Mundt

Reputation: 25281

The [] operator binds more closely than the unary * operator. Try changing *Z[i] to (*Z)[i] and see if your code behaves.

As a side note, it's also quite common in C to malloc a single array of (sizex*sizey) size, for a matrix and then index it as arr[x*sizey + y] or arr[y*sizex + x]. That more closely mimics what the language does with static arrays (e.g. if you declare int foo[10][10], all 100 ints are contiguous in memory and nowhere is a list of 10 int*'s stored.

Upvotes: 3

Related Questions