C - Heap Corruption Detected

Question

I got assigment from the professor to create a list using pointers. I'm having a problem with free() in all of my functions when I try to free memory. I keep getting a message:

"HEAP CORRUPTION DETECTED: after Normal block (#83) at 0x00D58CE0. CRT detected that the application wrote to memory after end of heap buffer."

I have no idea how to fix it. I've tried different things but nothing worked so far. I'm not sure where to search for it either anymore. I know that my program is not properly secured yet, but it doesn't affect the problem. I'm trying to eliminate it first before going further with it. Also, do you have any advice on detecting memory leaks from the program in visual studio? Here are my functions, full source code is below in the link:

Full Source Code

struct ListElement
{
    int value;
    struct element *next;
};

typedef struct ListElement List;
typedef List *ListEl;

void ViewListBackwards(ListEl *list_el)
{
    ListEl current_element = *list_el;
    int size = 0;
    int i = 0;
    int *reversed_array;
    while (current_element->next != NULL)
    {
        size++;
        current_element = current_element->next;
    }
    current_element = *list_el;
    reversed_array = (int*)malloc(size * sizeof(*reversed_array));
    for (i = size; i >= 0; i--)
    {
        reversed_array[i] = current_element->value;
        current_element = current_element->next;
    }
    for (i = 0; i <= size; i++)
    {
        printf(" %d. %d\n", i + 1, reversed_array[i]);
    }
    free(reversed_array);
}

void RemoveFromListFront(ListEl *list_el)
{
    if (ListEmpty(list_el) == 0)
    {
        ListEl current_element = *list_el;
        *list_el = current_element->next;
        free(current_element);
    }
    else
    {
        printf("List is empty!\n");
    }
}

void RemoveFromListBack(ListEl *list_el)
{
    if (ListEmpty(list_el) == 0)
    {
        ListEl current_element = *list_el;
        ListEl last_element = *list_el;
        while (current_element->next != NULL)
        {
            last_element = current_element;
            current_element = current_element->next;
        }
        last_element->next = NULL;
        free(current_element);
    }
}

[don't cast malloc](https://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc) — Barmar, Oct 13 '17 at 23:59
for finding memory leaks, Google _crtBreakAlloc. When you close your program, you should see an object dump in the Visual Studio output window if you have memory leaks. What you would then do, is add _crtBreakAlloc to your watch window, copy the leak memory location from the object dump and the debugger will break when that location gets allocated. Also watch out for the VS 2013->2015 version, the variable name is slightly different. — Nik, Oct 14 '17 at 00:02
`ListEl new_element = malloc(sizeof(ListEl));` --> `ListEl new_element = malloc(sizeof(*new_element));` — BLUEPIXY, Oct 14 '17 at 00:05
Debug it, `gdb` or in visual studio (no idea how but it certainly is possible). Find out which line is causing the crash — Erik W, Oct 14 '17 at 00:05
`struct element *next;` --> `struct ListElement *next;` first, It is recommended that you read the compiler warning. — BLUEPIXY, Oct 14 '17 at 00:08
`if (ListEmpty(list_el) == 0)` maybe --> `if (ListEmpty(*list_el) == 0)` and so on. — BLUEPIXY, Oct 14 '17 at 00:15
BLUEPIXY, you're right, I haven't noticed it. `struct element *next;` should be `struct ListElement *next` And `ListEl new_element = malloc(sizeof(ListEl));` should be `ListEl new_element = malloc(sizeof(List));` That fixed problems with removing from the front and back of the list. I still have problem with that array in the first function though. — Maissae, Oct 14 '17 at 00:23
`while (current_element->next != NULL){ size++;` : `size` is one less than the actual number of elements. --> `while (current_element != NULL){ size++;` — BLUEPIXY, Oct 14 '17 at 00:28
Erik W, debugging it in visual studio doesn't really give any good information about what's going on. I have only sudden information that application has triggered a breakpoint and in the debug window I have that note with "Heap Corruption Detected". The line which causes that is probably the `free()` function itself. Though breakpoint appears the next line after the `free()` function. [Screenshot](http://prntscr.com/gx6efx) — Maissae, Oct 14 '17 at 00:31
@Maissae BTW Read [Replying in comments](https://stackoverflow.com/editing-help#comment-reply) — BLUEPIXY, Oct 14 '17 at 00:43
@BLUEPIXY Thanks for the info. I'm still rather new to this site. I've did some changes to code, just like you said it was one less than number of elements. I've also removed the `for` since it caused some problems and used different apporach with `while`. This time everything works. Thanks for help! [Code](https://pastebin.com/ynmqHnGH) — Maissae, Oct 14 '17 at 00:58

Joey Pabalinas · Answer 1 · 2017-10-14T07:33:58.927

In the following code:

   reversed_array = (int*)malloc(size * sizeof(*reversed_array));
for (i = size; i >= 0; i--)
{
    reversed_array[i] = current_element->value;
    current_element = current_element->next;
}
for (i = 0; i <= size; i++)
{
    printf(" %d. %d\n", i + 1, reversed_array[i]);
}

you are allocating (what amounts to) an int reversed_array[size] array, but then proceed to write to reversed_array[size], which is sizeof(int) past the end of the allocated memory segment.

You instead want to change your for loops to:

for (i = size - 1; i >= 0; i--)

so that the indices you write to start at reversed_array[size - 1].

EDIT:

As an aside, please, as other people have suggested in the comments, don't cast malloc(). This is unneeded in C, as void * can be assigned to any object pointer type variables, and in addition can hide bugs in your code, such as forgetting to include stdlib.h.

And FWIW, you don't need the parentheses around sizeof(*reversed_array), as parentheses are only needed when applying the sizeof operator to types.

This is mostly just a style suggestion; it is preferred by many (myself included) that you omit the extraneous parentheses and write that expression as sizeof *reversed_array, as that makes it clear that sizeof is a unary operator and not a function.

C - Heap Corruption Detected

1 Answers1