0

When I try to free an allocation in a struct inside a struct, I get an error. How can I fix it?

typedef struct card
{
    char* sign;
    char* color;
    int number;
    char* name;
}card;

typedef struct deck
{
    card data;
    deck* next;

}deck;

deck* deleteHead(deck* head)
{ 
    deck* curr = head;
    if (head==NULL)
        return head;
    curr=curr->next;
    if(head->data.color!=NULL)
        free(head->data.color);//error
    if(head->data.name!=NULL)
        free(head->data.name);//error
    if(head->data.sign!=NULL)
        free(head->data.sign);//error
    free(head);//ok
    return curr;
}

when I'll delete the errors and only freeing the head - it'll work, but when I'll try to delete the allocations inside the head, I'll get a run time error. How can I solve this? Thank you in advance.

AvitalG
  • 1
  • 2
  • 2
    Please create an [mcve](http://stackoverflow.com/help/mcve)... – autistic May 30 '15 at 10:21
  • Does that even compile? I'd have though you'd have to `typedef struct deck {card data; struct deck *next}deck;`. Note the extra `struct`. – EOF May 30 '15 at 10:24
  • 1
    If I had to guess, I'd say you'd either never allocated the members of `card`, or you'd freed them already somewhere else. But without an mcve, we can't tell. – Baldrick May 30 '15 at 10:26
  • Could you also post how you allocate sign, color and name of your cards? – greyhairredbear May 30 '15 at 10:26

2 Answers2

1

You probably did not initialize the pointers in the card structure. These should either be initialized to NULL or to a pointer to memory allocated by malloc, calloc or strdup.

Also note that you don't need to test pointers against NULL before calling free(). free(NULL); will gracefully return immediately, it is legal to call free with NULL. Incidentally it is also legal in C++ to delete a null pointer.

The function can be further simplified this way:

deck *deleteHead(deck *head) { 
    deck *next = NULL;
    if (head != NULL) {
        next = head->next;
        free(head->data.color);
        free(head->data.name);
        free(head->data.sign);
        free(head);
    }
    return next;
}
chqrlie
  • 131,814
  • 10
  • 121
  • 189
-1

The function free can only de-allocate a block of memory previously allocated by a call to malloc, calloc or realloc. Your code will run without any runtime error if you initialize it properly. Here's a sample code:

int main()
{
    deck* root = (deck*)malloc(sizeof(struct deck));
    root->card.color = strdup("color");
    root->card.name = strdup("name");
    root->card.sign = strdup("sign");
    root->card.number = 2;
    root->next = NULL;
    root = deleteHead(root);
    return 0;
}

And also there is a slight correction in your code:

typedef struct deck
{
    card data;
    struct deck* next;

}deck;
iobleed
  • 91
  • 8
  • @chqrlie thank you for pointing it out. I was just giving an example. Kindly re-check my answer. – iobleed May 30 '15 at 12:31
  • You should not store pointers to character string constants to `char *` struct members. The C compiler tolerates this for historical reasons but your are effectively removing `const`ness, which should be flagged with a warning. Furthermore, since the OP deallocates these members with `free()`, it will invoke undefined behaviour. Use `c->color = strdup("color");`... – chqrlie May 30 '15 at 12:50
  • Okay. Use `strdup()` to allocate memory, for the new string, obtained from `malloc()` which can be freed using `free()` as in OP. Thanks again for the information. – iobleed May 30 '15 at 13:52
  • I'm sorry, I had not seen other problems: it is useless to allocate the `card` structure and copy it to the `deck` structure, directly initialize the `root->card.color` and other members. Furthermore, you do not initialize the `next` member in the `deck` pointed to by `root`. This makes `deleteHead()` invoke undefined behaviour. – chqrlie May 30 '15 at 13:57
  • so much for just an example, well all the version of this answer is working without even any warning. All the help is appreciated though. Thanks. – iobleed May 31 '15 at 20:26