2

I'm a newbie trying to learn how to make dynamics arrays in C. The code doesn't give me any errors when I build it using code:blocks, but when I run it crashes. I think the crash has to do with the way I'm freeing my memory, because the code is giving me the desired output before crashing.

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

int main()
{

    int i, j;
    int *p = (int *)malloc(sizeof(*p));

    printf("Hello World! I have created a dynamic array of 20x30 integers! \n");
    for (i = 0; i <= 19; i++)
    {
        p[i] = (int )malloc(sizeof(int*));
        printf(" %2d ", i);
        for (j = i + 1; j <= 29 + i; j++)
        {
        p[i] = 0;
        printf("%2d", j);
        }
    printf("\n");
    }

    for (i = 0; i <= 19; i++);
    {
        free(p[i]);
    }
    free(p);
    return 0;
}
Kasdu
  • 31
  • 2
  • 4
    Your first malloc allocates space for a single int. – tkausl Sep 24 '16 at 20:56
  • 2
    In addtion, `p[i] = (int )malloc(sizeof(int*));` is wrong. `p[i]` is an `int` and not a pointer so you should not store a pointer in there. On many systems the size of `int` and pointer are different and thus that code could also cause a crash. – kaylum Sep 24 '16 at 20:57
  • [fixed code](http://ideone.com/WWiYZL) – BLUEPIXY Sep 24 '16 at 21:36
  • No need to cast the result of malloc and friends in C, nor is it recommended in any way. Even more it may hide errors as in your case. Remove all those useless casts, recompile, fix the code until no more warnings are issued, be happy. :-) Also you want change this `malloc(sizeof(*p));` to allocated enough pointers, not just one. – alk Sep 25 '16 at 09:49

1 Answers1

0

Here's the problem.

First, your first malloc call allocates space for a 1-element array.

You'll want to change it from

int *p = (int *)malloc(sizeof(*p));

to

int *p = (int *)malloc(sizeof(int*) * 20);

And then your second malloc call is slightly incorrect as well.

p[i] = (int )malloc(sizeof(int*));

should be changed to

p[i] = (int *)malloc(sizeof(int));

You just put the asterisk in the wrong place.

Finally, you really only create a 20-element array. All you do in the inner for loop is assign each cell in the array the value 0 times. If you want to make a 20x30 array, you can always take the easy route and create a 1D array and use some math (which is ultimately what the compiler does with non-dynamic 2D arrays anyway):

int main()
{
    int *p = (int *)malloc(sizeof(int) * 600);
    ...
    for (i = 0; i <= 19; i++)
    {
        printf(" %2d ", i);
        for (j = 0; j <= 29; j++)
        {
            p[i * 30 + j] = 0; // It's i * 30, not i * 20 because you have to skip the space that the 'j' dimension takes up.
            printf("%2d", j);
        }
        printf("\n");
    }

    free((void*)p); //I found the program crashes without the void* cast
}

I've tested this code and it runs.

Hope this helps.

alk
  • 69,737
  • 10
  • 105
  • 255
anonymoose
  • 819
  • 2
  • 11
  • 25