Where is the segfault in this code?

Question

I believe the malloc is badly assigned, why?

 int ** array;
  int i,j;

  array=malloc(10*sizeof(int *));

  for(i=0; i<10; i++)
    for (j=0;j<10; j++)
        array[i][j]=i*j;

Answer 1

it should be malloc(10*10*sizeof(int));

there are two methods of creating 2d arrays in c: using continuous memory or using array of pointers. In first case you malloc 10*10 consecutive elements and access is done like this: array[i][j] = *(array + i*10 + j) = array[i*10 + j] (don't forget that hardcoding constants is smell). In other case you malloc 10 elements of type int* then you malloc rows in loop. then access like this array[i][j] = *(*(array + i) + j)

Answer 2

The problem is that your array is 10x10 = 100 elements, but you are mallocing only 10 elements' worth of space (incidentally, the comment above is correct: malloc for sizeof(int), not sizeof(int*) since int is what you actually want to store in the array).

If you change the malloc to

malloc(100 * sizeof(int))

then you should be fine.

EDIT: Just noticed that you are declaring it as int**. For a rectangular array like this you can declare it as int* and index by (j * 10 + i). Otherwise, you'll have to malloc the first dimension, then malloc each entry for the second dimension. This is slow and bug-prone, so better to use the j*10+i method.

Answer 3

You have a two-dimensional array so you need to allocate enough space to hold 100 elements. Alternatively, you need to allocate each column (per row) before you put things in that row. I'd only do the latter if the array were a jagged array, having different numbers of elements per row.

array=malloc(100*sizeof(int)); 

for(i=0; i<10; i++) 
  for (j=0;j<10; j++) 
    array[i*10+j]=i*j; 

or

array=malloc(10*sizeof(int *)); // rows

for(i=0; i<10; i++) {
  array[i] = malloc(10*sizeof(int));  // columns
  for (j=0;j<10; j++) 
      array[i][j]=i*j;
}

Answer 4

You've allocated the memory for your first dimension, but not the second.

size_t rows = 10;
size_t cols = 10;
int **array = malloc(sizeof *array * rows); // allocates array of 10 int *
if (array)
{
  size_t i;
  for (i = 0; i < rows; i++)
  {
    array[i] = malloc(sizeof *array[i] * cols); // allocates array of 10 int
    if (array[i])
    {
      size_t j;
      for (i = 0; i < cols; i++)
        array[i][j] = i * j;
    }
  }
}

Note that this does not allocate a contiguous block of memory; if you need all 100 elements to be contiguous, you have several choices:

Choice 1: Allocate a 1-D array and compute offsets manually as several others have shown;

Choice 2: Allocate a 1-D array and use an array pointer and some casting magic to make it look like a 2-D array:

#define COLS 10

size_t rows = 10;
size_t cols = COLS;
int *array = malloc(sizeof *array * rows * cols);
int (*parr)[COLS] = (int (*)[COLS]) array;
if (array)
{
  size_t i, j;
  for (i = 0; i < rows; i++)
    for (j = 0; j < cols; j++)
      parr[i][j] = i * j;
}

The drawback is that to declare the pointer you have to know how many columns you're working with, and for C89 you have to use a compile-time constant expression for the column dimension, which kind of defeats the purpose of dynamic allocation (using the cols variable wouldn't work unless you were using a C99 compiler).

Choice 3: If you're using C99, you can use a VLA and avoid malloc (and the accompanying free) altogether:

int rows = ROWS;
int cols = COLS;

int array[rows][cols];

for (int i = 0; i < rows; i++)
  for (int j = 0; j < cols; j++)
    array[i][j] = i * j;