2

I'm trying to allocate a 2d array in a C program. It works fine in the main function like this (as explained here):

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

int main(int argc, char ** argv)
{
    int ** grid;
    int i, nrows=10, ncols=10;
    grid = malloc( sizeof(int *) * nrows);

    if (grid == NULL){
        printf("ERROR: out of memory\n");
        return 1;
    }

    for (i=0;i<nrows;i++){
        grid[i] = malloc( sizeof(int) * ncols);
        if (grid[i] == NULL){
            printf("ERROR: out of memory\n");
            return 1;
        }
    }
    printf("Allocated!\n");

    grid[5][6] = 15;
    printf("%d\n", grid[5][6]);
    return 0;
}

But since I have to do this several times with different arrays, I was trying to move the code into a separate function.

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

int malloc2d(int ** grid, int nrows, int ncols){
    int i;
    grid = malloc( sizeof(int *) * nrows);

    if (grid == NULL){
        printf("ERROR: out of memory\n");
        return 1;
    }

    for (i=0;i<nrows;i++){
        grid[i] = malloc( sizeof(int) * ncols);
        if (grid[i] == NULL){
            printf("ERROR: out of memory\n");
            return 1;
        }
    }
    printf("Allocated!\n");
    return 0;
}

int main(int argc, char ** argv)
{
    int ** grid;

    malloc2d(grid, 10, 10);
    grid[5][6] = 15;
    printf("%d\n", grid[5][6]);
    return 0;
}

However, although it doesn't complain while allocating, I get segmentation fault when accessing the array. I read different posts on decayed arrays and similar topics, but I still can't figure out how to solve this problem. I imagine I'm not passing the 2d array correctly to the function.

Many thanks.

Improve this question
10
  • Generally, when you run out of memory, you should call the exit function. Best is to make a function that does that for you, say, xmalloc. Commented May 1, 2011 at 1:19
  • For what it's worth, it's usually a lot easier to avoid dynamically allocating multidimensional arrays; since multiple blocks have to be allocated it is less efficient than making a single allocation and doing the pointer arithmetic yourself. The code required to manage (allocate, reallocate, and deallocate) multidimensional arrays is also significantly more complex. Commented May 1, 2011 at 1:22
  • @James: Actually, in C it's impossible to allocate a multidimensional array. Multidimensional arrays are single memory blocks, with the multiplication to find the right location done as syntactic sugar done by the compiler to make it easier. (In C++ you actually can do this.... i.e. new int[5][6] ... I think, but I could be wrong there, in which case you can't do it in C++ either) Commented May 1, 2011 at 1:25
  • @Billy Yes, new int[5][6] works in C++, but I don't know how to write the int.... = new int[5][5] part. I have gotten as far as int* b[5] = new int[5][5]; but that gives the compiler error cannot convert from 'int (*)[5]' to 'int *[5]'. Tell me if you figure it out, I didn't know you could do this. Commented May 1, 2011 at 2:09
  • @Seth: You could always use auto. :) Commented May 1, 2011 at 2:19

7 Answers 7

Reset to default
8

That is not a multidimensional array; it is a single dimensional array containing pointers to single dimensional arrays. Multidimensional arrays do not contain pointers; they are single memory blocks.

Your problem here is that you have a pointer to a pointer, and you're trying to return it from your function through a parameter. If you're going to do that, you're going to need a pointer to a pointer to a pointer as your parameter, and you're going to have to pass the address of a pointer to a pointer to the method. If you don't do this, you're not changing the value of the variable grid in main -- you're changing the one which was copied as the parameter to the malloc2d function. Because the grid in main is left uninitialized, you get a undefined behavior.

Here's an example of what I mean as the fix:

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

int malloc2d(int *** grid, int nrows, int ncols){
    int i;
    *grid = malloc( sizeof(int *) * nrows);

    if (*grid == NULL){
        printf("ERROR: out of memory\n");
        return 1;
    }

    for (i=0;i<nrows;i++){
        (*grid)[i] = malloc( sizeof(int) * ncols);
        if ((*grid)[i] == NULL){
            printf("ERROR: out of memory\n");
            return 1;
        }
    }
    printf("Allocated!\n");
    return 0;
}

int main(int argc, char ** argv)
{
    int ** grid;

    malloc2d(&grid, 10, 10);
    grid[5][6] = 15;
    printf("%d\n", grid[5][6]);
    return 0;
}

Additional notes:

  • If a single allocation fails, you leak the allocation for the first array, as well as the allocations for all previous rows. You need to call free on those before returning.
  • You're returning through a parameter, even though you really don't have to. If I were writing this, I'd make the method return int **, and signal error by returning 0.
Improve this answer
Sign up to request clarification or add additional context in comments.

2 Comments

Thanks for the fantastic answer! I changed the function to return int ** as you also suggested.
(*grid) I didn't know that it can be used like that. It solved my problem too. Because I want to pass **A to a funcion and inside that function will know my size (cols and rows) and then write the information inside that function. then outside I want to refer to its values. Thanks a lot.
3

Here is your function, fixed:

int malloc2d(int *** grid, int nrows, int ncols){
    int i;
    *grid = (int**)malloc( sizeof(int *) * nrows);

    if (*grid == NULL){
        printf("ERROR: out of memory\n");
        return 1;
    }

    for (i=0;i<nrows;i++){

        (*grid)[i] = (int*)malloc( sizeof(int) * ncols);
        if ((*grid)[i] == NULL){
            printf("ERROR: out of memory\n");
            return 1;
        }
    }
    printf("Allocated!\n");
    return 0;
}

int main(int argc, char ** argv)
{
    int ** grid;

    malloc2d(&grid, 10, 10);
    grid[5][6] = 15;
    printf("%d\n", grid[5][6]);
    return 0;
}

Note the function now recieves an int*** and you pass the address of your int** to the function. The function then dereferences the int*** to put the address of the allocated memory block in it.

Improve this answer

3 Comments

That's not fixed. The if (grid == NULL){ and if (grid[i] == NULL){ lines also need fixed.
@Billy whoops, missed that. Fixed.
@Billy thanks, though your answer is more thorough and complete, and deserves the green check. +1 for your answer. Hopefully the asker will return and select it.
2

C is pass by value. And to sum up the error you are doing, this example should be helpful -

void foo( int *temp )
{
     temp = malloc(sizeof(int)) ;
     // temp is assigned to point to new location but the actual variable
     // passed from main do not point to the location temp is pointing to.

     *temp = 10 ;
}

int main()
{
     int *ptr ;
     foo( ptr ) ;

     // ptr is still unintialized
     *ptr = 5 ; // Segmentation fault or Undefined behavior

     return 0;
}

So, instead you should do -

void foo( int **temp )
{
    *temp = malloc(sizeof (int) );
    // ...
}

And now call the function as foo(&ptr); in the main() function.

Improve this answer

Comments

1

If you still want malloc2d to return a status code, the parameter needs to be of type int***:

int malloc2d(int *** grid, int nrows, int ncols){

And you need to use *grid to refer to the supplied buffer:

    int i;
    *grid = malloc( sizeof(int *) * nrows);

    if (*grid == NULL){
        printf("ERROR: out of memory\n");
        return 1;
    }

    for (i=0;i<nrows;i++){
        (*grid)[i] = malloc( sizeof(int) * ncols);
        if ((*grid)[i] == NULL){
            printf("ERROR: out of memory\n");
            return 1;
        }
    }
    printf("Allocated!\n");
    return 0;
}

Then, when calling malloc2d, pass it the address of an int** to fill in:

int ** grid;

malloc2d(&grid, 10, 10);
Improve this answer

4 Comments

I think you have one too many *s on your second to last int**** -- it should only be 3 *s.
@Billy ONeal: No, for int** grid, &&grid was of type int****.
@John: Ah.. where did &&grid come from? It's not in the OP's code.
@Billy ONeal: No idea. I probably mistyped it myself! Fixed, anyway.
0

C is actually always pass by value, so when you pass in 'grid' you are passing in a value and the function is modifying its own local copy. Try passing in '&grid' instead and modify malloc2d appropriately.

Improve this answer

1 Comment

The problem is larger than just adding the &. First of all, the compiler won't allow it due to the wrong level of indirection, and second, the function is still wrong after you do it.
0

That's not your about your segmentation fault problem, but you should really consider to use a single malloc call to allocate all necessary memory space need by grid.

grid = malloc (nrows * ncols * sizeof(int *))

Look at Bill ONeary answer regarding pointer of pointer of pointer.

Improve this answer

3 Comments

While I agree with this, it really doesn't answer the OP's question.
@Billy Oneal: you are right but he is too much complicating the code instead of using a simple solution.
I agree, but you have not posted an answer to the OP's question. This content reallly belongs in a comment if it's not going to answer the original question.
0

Shorter should be below, with free (!) and initialized values for each grid element:

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

#define NROWS 10
#define NCOLS 10

int main()
{
    int (* grid)[NCOLS] = calloc(NROWS,sizeof*grid);

    /* no more needed here malloc2d(grid, 10, 10); */
    grid[5][6] = 15;
    printf("%d\n", grid[5][6]);
    free(grid); /* every c/malloc need a free */
    return 0;
}
Improve this answer

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.