Dynamic memory allocation for two-dimensional arrays

Question

I want to allocate memory for a two-dimensional array (matrix) and write the sums of the diagonals in a separate one-dimensional array. So my code has an array of pointers to pointers,

int N, ** matrix = NULL;
matrix = (int**) malloc(sizeof(int*) * N);

I fill it and then I create an array to store the sums of the diagonals,

int diag = 2 * N - 1;
int *diagonals = NULL;
diagonals = (int*)malloc(sizeof(int) * diag);

but when I want to write a value into an array, something goes wrong, the values just don't get written into the array; I don't know why.

Here is my code:

#include <stdio.h>
#include <time.h>
#include <stdlib.h>
int main() {
    srand(time(NULL));
    int N, ** matrix = NULL;
    printf("Input the number of rows\n");
    scanf_s("%d", &N);
    printf("\n");
    // Memory allocation for the array of pointers to pointers
    
    matrix = (int**) malloc(sizeof(int*) * N);
    if (matrix != NULL)
    {
        for (int i = 0; i < N; i++)
            *(matrix + i) = (int*)malloc(sizeof(int) * N);

        for (int i = 0; i < N; i++)
        {
            for (int j = 0; j < N; j++)
            {
                matrix[i][j] = rand() % 14 - 4;
                printf("%d\t", matrix[i][j]);
            }
            printf("\n");
        }
        printf("\n");

        int diag = 2 * N - 1;
        int *diagonals = NULL;
        diagonals = (int*)malloc(sizeof(int) * diag);

            for (int i = 0; i < N; i++)
            {
                for (int j = 0; j < N; j++)
                {
                    diagonals[i+j] += matrix[i][j];;
            }
        }

            for (int i = 0; i < diag; i++) {
                printf("diagonals[%d] - %d\n",i, *(diagonals+i));
        }

    }
    else
        printf("Not enough memory.. oops..\n");
}

`*(matrix + i)` is a really smug way of writing `matrix[i]`, IMO. — Marco, Nov 29 '22 at 22:16
You did not initialize values in the `diagonals` array before the loops. Those loops use `+=` to modify the _existing_ value. You're clearly assuming the values start off initially as zero, but they are uninitialized. You must initialize them explicitly with a loop, or `memset`, or allocate with `calloc`. — paddy, Nov 29 '22 at 22:16
And since this is tagged `c`, you [don't cast](https://stackoverflow.com/a/605858/2005038) the result of `malloc`. — Marco, Nov 29 '22 at 22:17
You could save yourself quite a lot of code by declaring and allocating `matrix` with a simple `int (*matrix)[N] = malloc(N * sizeof *matrix);` — tstanisl, Nov 29 '22 at 22:31
@tstanisl I'd go even further. Since all the rows are same length, I'd just use a flat 1D array and some simple arithmetic when indexing. — Dan Mašek, Nov 29 '22 at 22:43
@DanMašek, maybe, but manual calculations of indices becomes very cumbersome for 3D and more tensors. VLA **types** were introduced to make handling such entities much easier. — tstanisl, Nov 29 '22 at 22:45
@marco-a Nah it's just a needlessly unreadable way. The really smug way would be `i[matrix]`. — Lundin, Nov 30 '22 at 09:09
So is this supposed to print all the diagonals of a matrix or calculate the sum of them? I'm not sure if it does either of that. — Lundin, Nov 30 '22 at 09:27
@Lundin I agree. It's not smug but needlessly unreadable... wasn't exactly sure how to call it — Marco, Nov 30 '22 at 21:18

score 2 · Accepted Answer · answered Nov 29 '22 at 22:21

2

The content of diagonals is allocated with malloc() which does not initialize the memory. You should use calloc() which initializes the memory with zeros:

diagonals = calloc(diag, sizeof *diagonals);

answered Nov 29 '22 at 22:21

tstanisl

13,520
2
25
40

score 2 · Answer 2 · answered Nov 29 '22 at 22:21

The following loop assumes that each element of diagonals was initialized to zero, but you performed no such initialization. As a result, they are uninitialized and will contain whatever value happens to be sitting in the newly-allocated memory.

diagonals = malloc(sizeof(int) * diag);
for (int i = 0; i < N; i++)
{
    for (int j = 0; j < N; j++)
    {
        diagonals[i+j] += matrix[i][j];;
    }
}

You have several options for zero-initialization:

Use memset to zero the memory, after allocation:

 diagonals = malloc(sizeof(int) * diag);
 memset(diagonals, 0, sizeof(int) * diag);

Initialize values in a loop:

 diagonals = malloc(sizeof(int) * diag);
 for (int i = 0; i < diag; i++) diagonals[i] = 0;

Allocate with calloc:

 diagonals = calloc(diag, sizeof(int));

Note that in all cases, you should be checking the result of allocation. If it fails and returns NULL, you should not attempt to access memory via that pointer.

score 0 · Answer 3 · answered Nov 29 '22 at 23:00

0

It does not answer the question but I would suggest using real 2D arrays instead of arrays of pointers. To dynamically allocate the 2D array you need to use a pointer to array.

    int (*matrix)[N] = malloc(N * sizeof(*matrix));

You will have only one allocation and one free. Fever levels of indirection.

answered Nov 29 '22 at 23:00

0___________

60,014
4
34
74

The use of VLA should include a test for availability, since its support is optional in C.2011 and above. See: https://stackoverflow.com/questions/49988521/check-support-for-variable-length-array-in-c – jxh Nov 30 '22 at 16:17
@jxh, support for VLA typew is going to be mandatory in C23. There will be no need for this check anymore – tstanisl Dec 02 '22 at 08:15
@tstanisl The references I have found refer to variably-modified types being mandatory, not VLA. Is that what you meant? – jxh Dec 02 '22 at 08:53
@jxh, VLA means an object of "VLA type" which is a *subtype* of VMT family. In C23 one can create any VLA type: `typedef int T[n];`. However, only creation of *an object* of VLA type with **automatic** storage are going to be optional. So `T a;` will be optional while support for `T *a = malloc(sizeof *a);` will be mandatory. – tstanisl Dec 02 '22 at 09:59
@tstanisl *VLA means an object of "VLA type"* -- got it, thanks. – jxh Dec 02 '22 at 15:56

Dynamic memory allocation for two-dimensional arrays

3 Answers3