CODEWITHSUNDEEP
cfreeheap-memorydynamic-arraysheap-corruption
Maissae
Maissae

Reputation: 1

C - Heap Corruption Detected

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);
    }
}

Upvotes: 0

Views: 2049

Answers (1)

Joey Pabalinas
Joey Pabalinas

Reputation: 126

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.

Upvotes: 1

Related Questions