Freeing memory of struct array

Question

I have two structures:

struct product {
    char *title;  // Name of the product
    char code[8]; // Max. 7 characters of product ID
    int stock;  // Current stock (number of units)
    double price;  // Price of a single unit
};

struct product_array {
    struct product *arr;
    unsigned int count;
};

I am adding products to product_array with function:

void add_product(struct product_array *pa, const char *title, const char *code,
        int stock, double price) {
    pa->count++;
    struct product* nProduct = malloc(sizeof (struct product));
    if (!nProduct) free(nProduct);
    init_product(nProduct, title, code, stock, price); 

    pa->arr = realloc(pa->arr, (pa->count) * sizeof (struct product));
    if (!pa->arr) free(pa->arr);

    pa->arr[pa->count - 1] = *nProduct;
}

void init_product(struct product *pr, const char *title, const char *code,
        int stock, double price) {
    int titleLen = strlen(title);
    int codeLen = strlen(code);
    char *aTitle = calloc((1 + titleLen) * sizeof (char), 1);
    strncpy(aTitle, title, titleLen);
    char* codePtr = strncpy(pr->code, code, codeLen);
    if (codeLen <= 7)
        *(codePtr + codeLen) = 0;
    else
        *(codePtr + 7) = 0;
    pr->title = aTitle;
    pr->stock = stock;
    pr->price = price;
}

add_product works like this in main.c

struct product_array pa;
pa.count = 0;
pa.arr = NULL;

struct product p;
init_product(&p, "test", "0000", 1, 0.50);

print_products(&pa);


add_product(&pa, "Product 1", "0000", 0, 10);
add_product(&pa, "Long name, isn't it", "1234567890", 10, 100);
add_product(&pa, "Product 3", "9999999", 0, 20);

print_products(&pa);
remove_all(&pa);

When I am trying to free all allocated memory, I run to problems. Here is remove all function:

int remove_all(struct product_array *pa) {
    unsigned int i;
    unsigned int until = pa->count;
    struct product *prdPtr = pa->arr;
    struct product *next;
    for (i = 0; i < until; i++) {
        next = prdPtr + 1;
        free(prdPtr->title);
        free(prdPtr);  // this raises error
        prdPtr = next;
    }
    if (pa->arr != NULL) {
        free(pa->arr);
    }    
    pa->count = 0;
    return 1;
}

The for-loop overflows, but I am now trying to figure out the logic of memory freeing in this context. In remove_all function I want to free all memory of product_array. I am iterating through every (struct)product in array and freeing the memory of title. After freeing memory of title I am trying to free the struct product itself. It works in first iteration, but when I come to second element, the title could be freed but free(prdPtr) raises SIGABRT.

What might I be missing? Why am I able to free the titles of products but not products themselves? Thank you for help in advance.

That seems to work, thank you! Now I understand the error, the array was allready freed.. — samu
– samu, Commented Apr 2, 2015 at 10:55

Klas Lindbäck · Accepted Answer · 2015-04-02 11:56:46Z

2

The pa->arr is struct product * not struct product **. So you have allocated memory for pa->arr and you should free it only once. pa->arr[i] is not a pointer, but just a structure.

You shouldn't free it, but you should free any memory allocated to its members like title.

So update your for loop as

...
for (i = 0; i < until; i++) {
        next = prdPtr + 1;
        free(prdPtr->title); //just free members
        prdPtr = next;
    }
if (pa->arr != NULL) {
    free(pa->arr);
}    
...

Edit:

Also note that there is an unnecessary malloc in add_product. Suggested fix:

void add_product(struct product_array *pa, const char *title, const char *code,
        int stock, double price) {
    struct product *pa_tmp;
    pa->count++;
    pa_tmp = realloc(pa->arr, (pa->count) * sizeof (struct product));
    if (pa_tmp == null) {
        /* handle out of memory error */
    }
    pa->arr = pa_tmp;
    init_product(&pa->arr[pa->count - 1], title, code, stock, price); 
}

edited Apr 2, 2015 at 11:56

Klas Lindbäck

33.4k5 gold badges61 silver badges85 bronze badges

answered Apr 2, 2015 at 10:59

Rohan

53.5k12 gold badges95 silver badges89 bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

Klas Lindbäck Over a year ago

@user3121023 Yes. In fact, prdPtr isn't needed either. You could just free(pa->arr[i].title);.

harper · Accepted Answer · 2015-04-02 10:55:39Z

0

You free the same Memory block twice:

// this makes pdrPtr to an alias of pa->arr
struct product *prdPtr = pa->arr;

for (i = 0; i < until; i++) {
    ...
    free(prdPtr);  // free prdPtr a.k.a. pa->arr
    ...
}
    // here you free the same oject again.
    free(pa->arr);

Remove the last free(pa->arr);.

answered Apr 2, 2015 at 10:55

harper

13.8k9 gold badges63 silver badges112 bronze badges

Collectives™ on Stack Overflow

Freeing memory of struct array

2 Answers 2

1 Comment

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

1 Comment

Comments

Your Answer

Sign up or log in

Post as a guest

Related