free() on a struct array

Question

I'm having trouble with my first program using malloc. My problem is that the program crashes when it executes the free() line. I have no idea why this is happening and would like to know how to prevent it from happening.

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

struct product{
    int cost;

    char thing[20];
};


int main()
{
    int amount;
    scanf("%d", &amount);
    getchar();
    struct product *products;
    products = (struct product *) malloc(amount);
    for (int i = 0; i < amount; i++)
    {
        printf("Thing of %d ", (i + 1));
        gets(products[i].thing);
        printf("Cost of %d: ", (i + 1));
        scanf("%d", &products[i].cost);
        getchar();
    }
    free(products);
    return 0;
}

score 7 · Answer 1 · answered Dec 12 '14 at 02:03

7

You're not allocating enough memory. It should be:

products = (struct product *) malloc(amount * sizeof(struct product));

(malloc cast left in from original code, I'm not entering that debate.)

answered Dec 12 '14 at 02:03

James M

18,506
3
48
56

It should [better](http://stackoverflow.com/q/605845/296974) be `products = malloc(amount * sizeof(struct product));`. A good alternative would be `products = malloc(amount * sizeof(*products));`. – glglgl Dec 13 '14 at 10:26
@glglgl The malloc casting is a never-ending argument here on SO, so I decided to stay out of it and just answer the question. – James M Dec 14 '14 at 19:26

Dinal24 · Answer 2 · 2014-12-12T04:43:34.283

Actually you malloc(), amount of memory and uses amount * sizeof(struct product). It might work fine, but when you call free() it causes the crash as you have written to some un-allocated memory and free() tries to free memory which is not actually not allocated for you.

products = malloc(amount * sizeof(struct product)); // No casting too

There is no need to cast the value returned by malloc() since it gets casted implicitly from a void* the pointer to someOther*.

2501 · Answer 3 · 2014-12-12T10:50:30.720

1

Besides not allocating enough memory with malloc(), the function gets() is unsafe, deprecated and therefore should never be used.

If at any point used enters more characters than the buffer has available, you will get undefined behavior. Replace it with fgets() which allows you to specify the buffer size.

edited Dec 12 '14 at 10:50

answered Dec 12 '14 at 07:02

2501

25,460
4
47
87

free() on a struct array

3 Answers3