Allocating a 2D array of structures

Question

I want to allocate a 2D array of my custom type "cell", which is a structure. However, I am doing something wrong, see my code below. Could you please tell me where my mistake is?

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

typedef struct
{
  int variable_1;
  int variable_2;
  int variable_3;
} cell;


void initialiseArray(unsigned long rows, unsigned long columns, cell array[rows][columns])
{
  for (int i = 0; i < rows; i = i + 1)
    for (int j = 0; j < columns; j = j + 1)
    {
      array[i][j].variable_1 = 0;
      array[i][j].variable_2 = 0;
      array[i][j].variable_3 = 0;
    }
}

int main()
{
  unsigned long rows = 200;
  unsigned long columns = 250;
  cell* array[rows];
  for (unsigned long i = 0; i < rows; i = i + 1)
    array[i] = malloc(columns * sizeof(cell));


  if (array == NULL)
  {
    printf("Error in ""main"": Memory could not be allocated.\n");
    return 3;
  }

  initialiseArray(rows, columns, array);

  return 0;
}

The error `array == NULL` check doesn't make sense as it's a stack allocated variable. You should check if `array[i]` is NULL though. — Allan Wind, May 21 '23 at 01:45
It's weird that you use a vla (i.e. stack allocated) for the row pointers but dynamically (heap) the columns. Do one or the other. — Allan Wind, May 21 '23 at 02:02

Allan Wind · Accepted Answer · 2023-05-21T06:41:54.020

cell* array[rows]; is a variable length array (VLA) that is allocated on the stack; each row is allocated on the heap with array[i] = malloc(columns * sizeof(cell));.

I suggest you either allocate the array on the stack or heap:

Stack:

    cell array[rows][columns];
    initialiseArray(rows, columns, array);

You cannot initilaize VLAs, btw, so you still need initialiseArray() (or use memset()). On Linux the default stack size is 8 MB, and if you try to use more than that your program will probably segfault.

Heap:

    cell (*array)[rows][columns] = malloc(sizeof *array);
    initialiseArray(rows, columns, *array);

As @yano mentioned you can use calloc() to zero initialize your memory so you don't necessarily need to call initialiseArray(). On Linux the limit is whatever amount of free physical memory + swap (say, 16 GB), and if you try to use more than available malloc() / calloc() will return NULL which you can easily handle.

Either is in fine in this case. If rows and columns are only known at run-time prefer heap allocation.

Prefer passing variable instead of a type of sizeof.

While i and j are perfectly valid loop variables consider using r / row or c / col / column in this case.

yano · Answer 2 · 2023-05-21T01:52:55.847

Take a look at the warnings, you have a number of issues, some more serious that others. The most egregious violation is the array argument of initialiseArray. When you pass an array to a function as an argument, it decays to a pointer to the first element. You have an array of cell pointer types, or cell* types. So a pointer to this type is a cell** type, so that's what you should change the argument to:

void initialiseArray(unsigned long rows, unsigned long columns, cell** array)

Additional warnings that you should fix:

Comparing int to unsigned long in initialiseArray
Checking if (array == NULL) makes no sense, it's an array in automatic storage, it will never be NULL. Instead, you mean to check if (array[i] == NULL), so move that check into the loop that's doing the mallocing.

void initialiseArray(unsigned long rows, unsigned long columns, cell** array)
{
  for (unsigned long i = 0; i < rows; i = i + 1)
    for (unsigned long j = 0; j < columns; j = j + 1)
    {
      array[i][j].variable_1 = 0;
      array[i][j].variable_2 = 0;
      array[i][j].variable_3 = 0;
    }
}

int main(void)
{
  unsigned long rows = 200;
  unsigned long columns = 250;
  cell* array[rows];
  for (unsigned long i = 0; i < rows; i = i + 1)
  {
    array[i] = calloc(sizeof(cell), columns);
    if (array[i] == NULL)
    {
        printf("Error in ""main"": Memory could not be allocated.\n");
        return 3;
    }
  }

  initialiseArray(rows, columns, array);

  return 0;
}

But since you're initializing your memory to 0, there's really no need for the initialiseArray function at all, simply allocate with calloc:

for (unsigned long i = 0; i < rows; i = i + 1)
{
  array[i] = calloc(sizeof(cell), columns);
  if (array[i] == NULL)
  {
    printf("Error in ""main"": Memory could not be allocated.\n");
    return 3;
  }
}

Now your memory is zeroized, no need to roll your own function.

Finally, don't forget to clean up your allocated memory if you so choose

Allocating a 2D array of structures

2 Answers2