# Where is the segfault in this code?

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;
``````

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;
}
``````

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)`

The problem is that your array is 10x10 = 100 elements, but you are `malloc`ing 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.

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;
``````