Heap corruption when freeing allocated memory (C)

Question

I'm allocating memory dynamically to maintain a list of items, but when trying to remove an item using free(), I get a memory heap corruption error. And I know this would be a lot easier to do in C++ (or java, or any other object-oriented language actually), but I have to do this in C (note: C code, but compiled in Microsoft Visual Studio 2010, and thus a C++ compiler).

Here is what the item "looks" like:

//item.h
typedef struct 
{
    char* titel;
    char* auteur;
    int jaar;
} Boek;

typedef struct
{
enum {BOEK, TIJDSCHRIFT} itemType;
    union {
        Boek* boek;
        Tijdschrift* tijdschrift;
    } itemData;
    struct Item* next;
} Item;

Here is how it is created/allocated. I've also switched between using strdup and malloc/strcpy for allocating the strings, but that didn't seem to have any effect. Also, the item I'm trying to remove is of the BOEK type, the TIJDSCHRIFT allocator/deallocator works in a similar way though.

//item.c
Item* nieuwBoek(char* _titel, char* _auteur, int _jaar)
{
    Item* item = (Item*) malloc(sizeof(Item*));
    item->itemType=BOEK;
    item->itemData.boek=(Boek*) malloc(sizeof(Boek*));
    item->itemData.boek->titel=strdup(_titel);
    item->itemData.boek->auteur=strdup(_auteur);
    item->itemData.boek->jaar=_jaar;
    item->next=NULL;
    return item;
}

The returned pointer is then used by another function which passes it to the item->next of the previous item in the list.

Here is how I'm trying to free it. It is my understanding that I'll have to free the allocated strings before freeing the struct itself, but even when I just call free(item) (either by commenting out the other code in deleteItem or by calling it in the main code: free(nieuwBoek(...) I get the heap corruption error.

void deleteItem(Item* item)
{
    if (item->itemType==BOEK)
    {
        free(item->itemData.boek->titel); // When not running in debug-mode it crashes here.
        free(item->itemData.boek->auteur);
        free(item->itemData.boek); // When running in debug mode it crashes here.
    }
    /*else if... TIJDSCHRIFT deallocator here*/
    free(item); 
}

And the pointer passed to deleteItem() is a valid pointer pointing to an item. Its probably something incredibly stupid I'm doing wrong/missing, but I've been stumped over this problem all day now, so I'm asking you guys for help. Oh, and the next->pointer is set to NULL prior to the deleting of an item, so it's already disconnected from the list, if that matters.

Answer 1

Item* item = (Item*) malloc(sizeof(Item*));

You should change that sizeof to sizeof(Item) or sizeof(*item). Otherwise you'll allocate just enough to hold a pointer, not nearly enough for your structure.

Personally I prefer sizeof *item - that way if I ever change its type, I only have to do it in one place.

---

Side notes:

• What the corruption means: since you allocated to little memory, when you filled your struct fields to messed up the internal bookkeeping of malloc and the next operation detected it
• Although a matter of personal preference, you should probably not cast the return of malloc

Answer 2

This:

Item* item = (Item*) malloc(sizeof(Item*));

only allocates enough space for a pointer. You want to allocate enough space for an entire Item structure:

Item* item = (Item*) malloc(sizeof(Item));

Similarly this alloc:

item->itemData.boek=(Boek*) malloc(sizeof(Boek*));

Should be:

item->itemData.boek=(Boek*) malloc(sizeof(Boek));

Answer 3

Late post to the party, but this type of problem is detected with Valgrind. I had this problem and it was driving me crazy as GCC would allow the program to run but VC2012 would crash every time. Valgrind showed the following:

    Invalid write of size 8
        at 0x10CC33: appendListItem (structMgmt.c:170)
        by 0x10BB6E: parse (parse.c:304)
        by 0x10A790: runParsingTests (CTSLicense.c:562)
        by 0x109B7F: main (CTSLicense.c:41)
    Address 0x5207748 is 0 bytes after a block of size 8 alloc'd
        at 0x4C2CB3F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
        by 0x10CC2A: appendListItem (structMgmt.c:169)
        by 0x10BB6E: parse (parse.c:304)
        by 0x10A790: runParsingTests (CTSLicense.c:562)
        by 0x109B7F: main (CTSLicense.c:41)

which was very confusing (I've known C for 2 weeks now). I had accidentally had a malloc for the pointer instead of the struct. Now my program runs in GCC and VC2012 without any trouble.

Using Microsoft's pageheap detection tool was not very helpful in finding the root cause of the problem. It did tell me there was heap corruption instead of the other stupid error that was showing up, but not the source of the problem.

Microsoft's heap detection tool

Hints to using valgrind to find this problem