Error with a dynamically allocated 2D array

Question

I have created a small piece of code to dynamically allocate a 2D array in C, this is being used to try and solve a problem I am having on a larger piece of code, if I can get my head around dynamically allocating a 2D array I am confident I will solve my other problem. The issue I have had is that after I have allocated the matrix and wrote to it with a function my program does not run, I do not get any errors, it just creates a blank screen and eventually crashes. I am not sure where to progress from here, any help would be much appreciated!

Here is the code:

#include <stdlib.h>

void get_matrix(double **a, int n);

int main() {
    int n = 5;
    int i, j;
    double **a;

    a = (double **)malloc(n * sizeof(double *));
    for (j = 0; j < n; j++)
        a[j] = (double *)malloc(n * sizeof(double));

    get_matrix(a, n);

    for (i = 0; i <= n; i++) {
        for (j = 0; j <= n; j++) {
            printf("%d, ", a[i][j]);
        }
        printf("\n, ");
    }
    return 0;
}

void get_matrix(double **a, int n) {
    int i, j;

    for (i = 0; i <= n; i++) {
        for (j = 0; j <= n; j++) {
            a[i][j] = 4;
        }
    }
}

Have you read [this](https://stackoverflow.com/q/42094465/694733)? — user694733, May 29 '17 at 10:35
@DavidBowling: strictly speaking, you are correct. Why not post an answer with c99 code that does allocate and use a 2D VLA? The syntax is somewhat more complicated than the 2D indirect array code posted above. — chqrlie, May 29 '17 at 10:47
@DavidBowling: There were some other problems, I posted a more complete answer. — chqrlie, May 29 '17 at 10:58

unalignedmemoryaccess · Answer 1 · 2017-05-29T10:38:49.293

2

Problem is in for loop. Your for loop now runs n + 1 times instead n times. This means you are trying to write somewhere on unallocated memory region.

for (i=0; i<=n; i++)

should be:

for (i=0; i<n; i++)

You have to use conditions is less instead of is less or equal.

While I can see your code, I would also suggest some tricks to avoid later problems by using variables in sizeof directly:

a = (double **) malloc (n * sizeof(*a));

This will automatically detect sizeof of *a which is required for allocation size.

edited May 29 '17 at 10:38

answered May 29 '17 at 10:33

unalignedmemoryaccess

7,246
2
25
40

That's it sorted thank you! You've solved a head ache! :) – Steven Taggart May 29 '17 at 10:46
Glad to hear ;) If it works for you, you may click "Correct answer" to confirm it. – unalignedmemoryaccess May 29 '17 at 10:47

chqrlie · Accepted Answer · 2017-06-25T21:08:44.920

2

There are multiple problems in your code:

You do not include <stdio.h>
Your loop indexes i and j run one step too far: since indexing is zero based in C, you must stop before n or you will try and access elements outside the array. This bug causes undefined behavior, a plausible explanation for what you observe.
You pass a double for the %d printf conversion specification. Use %g instead.
You print extra commas at the start of lines.
Your array is not really a 2D array, it is an indirect array. In C99, you can allocate and use dynamic arrays with parametric sizes, called VLAs. Look at the syntax below:

Here is an improved version:

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

static void get_matrix(int n, double a[n][n]) {
    for (int i = 0; i < n; i++) {
        for (int j = 0; j < n; j++) {
            a[i][j] = 4;
        }
    }
}

int main(void) {
    int n = 5;
    double (*a)[n] = malloc(sizeof(*a) * n);

    get_matrix(n, a);

    for (int i = 0; i < n; i++) {
        for (int j = 0; j < n; j++) {
            printf("%g, ", a[i][j]);
        }
        printf("\n");
    }
    return 0;
}

edited Jun 25 '17 at 21:08

answered May 29 '17 at 10:56

chqrlie

131,814
10
121
189

You beat me to it! Just to nitpick, I like to use `malloc(sizeof *a * n)`, placing the `sizeof` expression first by habit. This reduces the chance of overflow when you have more than one multiplication: `malloc(sizeof *a * x * y)`. And I would use `size_t` for `n` and the array indices. – ad absurdum May 29 '17 at 11:01
1

@DavidBowling: Good point, I usually do the same for exactly the same reason, but I prefer `sizeof(*a) * n` for clarity. I was caught once by `malloc(x * y * sizeof(*p))` with `x` and `y` integers smaller than `size_t`, the overflow is not easy to spot even staring at the code. With a single multiplication, it is not an issue, but consistency is a golden rule. – chqrlie May 30 '17 at 06:50
Hi there! I have not been able to work on this problem since you last helped me out last week! I have made some changes to how it was written and it runs fine, the only problem being that when I print the matrix it only prints zeros? – Steven Taggart Jun 03 '17 at 19:58
@StevenTaggart: did you change the `printf` format as noted in my answer? – chqrlie Jun 03 '17 at 21:42
That's it sorted @chqrlie Thanks again! :) – Steven Taggart Jun 03 '17 at 22:04

score 1 · Answer 3 · answered May 29 '17 at 10:36

Your indexes are zero based. This means you check the end condition of a for-loop with a < and not a <=. 0..n-1 are the n locations you allocated.

#include <stdlib.h>

void get_matrix (double **a, int n);

int main ()

{
    int n = 5;
    int i, j;
    double **a;



    a = (double **) malloc (n * sizeof(double *));
    for (j = 0; j < n; j++)
        a[j] = (double *) malloc (n * sizeof(double));

    get_matrix (a, n);

    for (i=0; i<n; i++)
    {
        for (j=0; j<n; j++)
        {
            printf("%d, ", a[i][j]);
        }
        printf("\n, ");
    }

    return 0;

}

void get_matrix (double **a, int n)
{
    int i, j;

    for (i=0; i<n; i++)
    {
        for (j=0; j<n; j++)
        {
            a[i][j] = 4;
        }
    }
}

This code has undefined behavior since `a[i][j]` is a `double`, being printed with a `%d` conversion specifier. — ad absurdum, May 29 '17 at 12:38

Error with a dynamically allocated 2D array

3 Answers3