HEAP CORRUPTION DETECTED: after normal block(#87)

Question

I'm trying to do a program that get number of names from the user, then it get the names from the user and save them in array in strings. After it, it sort the names in the array by abc and then print the names ordered. The program work good, but the problem is when I try to free the dynamic memory I defined. Here is the code:

#include <stdio.h>
#include <string.h>

#define STR_LEN 51

void myFgets(char str[], int n);
void sortString(char** arr, int numberOfStrings);

int main(void)
{
    int i = 0, numberOfFriends = 0, sizeOfMemory = 0;
    char name[STR_LEN] = { 0 };
    char** arrOfNames = (char*)malloc(sizeof(int) * sizeOfMemory);
    printf("Enter number of friends: ");
    scanf("%d", &numberOfFriends);
    getchar();
    for (i = 0; i < numberOfFriends; i++)  // In this loop we save the names into the array.
    {
        printf("Enter name of friend %d: ", i + 1);
        myFgets(name, STR_LEN);  // Get the name from the user.
        sizeOfMemory += 1;
        arrOfNames = (char*)realloc(arrOfNames, sizeof(int) * sizeOfMemory);  // Change the size of the memory to more place to pointer from the last time.
        arrOfNames[i] = (char*)malloc(sizeof(char) * strlen(name) + 1);  // Set dynamic size to the name.
        *(arrOfNames[i]) = '\0';  // We remove the string in the currnet name.
        strncat(arrOfNames[i], name, strlen(name) + 1);  // Then, we save the name of the user into the string.
    }
    sortString(arrOfNames, numberOfFriends);  // We use this function to sort the array.
    for (i = 0; i < numberOfFriends; i++)
    {
        printf("Friend %d: %s\n", i + 1, arrOfNames[i]);
    }
    for (i = 0; i < numberOfFriends; i++)
    {
        free(arrOfNames[i]);
    }
    free(arrOfNames);
    getchar();
    return 0;
}

/*
Function will perform the fgets command and also remove the newline
that might be at the end of the string - a known issue with fgets.
input: the buffer to read into, the number of chars to read
*/
void myFgets(char str[], int n)
{
    fgets(str, n, stdin);
    str[strcspn(str, "\n")] = 0;
}

/*In this function we get array of strings and sort the array by abc.
Input: The array and the long.
Output: None*/
void sortString(char** arr, int numberOfStrings)
{
    int i = 0, x = 0;
    char tmp[STR_LEN] = { 0 };
    for (i = 0; i < numberOfStrings; i++)  // In this loop we run on all the indexes of the array. From the first string to the last.
    {
        for (x = i + 1; x < numberOfStrings; x++)  // In this loop we run on the next indexes and check if is there smaller string than the currnet.
        {
            if (strcmp(arr[i], arr[x]) > 0)  // If the original string is bigger than the currnet string.
            {
                strncat(tmp, arr[i], strlen(arr[i]));  // Save the original string to temp string.
                // Switch between the orginal to the smaller string.
                arr[i][0] = '\0';
                strncat(arr[i], arr[x], strlen(arr[x])); 
                arr[x][0] = '\0';
                strncat(arr[x], tmp, strlen(tmp));
                tmp[0] = '\0';
            }
        }
    }
}

After the print of the names, when I want to free the names and the array, in the first try to free, I get an error of: "HEAP CORRUPTION DETECTED: after normal block(#87)". By the way, I get this error only when I enter 4 or more players. If I enter 3 or less players, the program work properly. Why does that happen and what I should do to fix it?

You typically get such an error when you go out of bounds of your allocated memory. — Some programmer dude, Apr 30 '21 at 08:18
And `char** arrOfName` when you use `sizeof(int)` as the element size? That doesn't make much sense. Neither does the cast `(char*)` — Some programmer dude, Apr 30 '21 at 08:19
Why do you `realloc` `arrOfNames` inside the loop, when you know exactly how much memory it needs right after the `scanf`? — Michael, Apr 30 '21 at 08:20
Heap corrupt means check malloc before anything else. Does it make sense? `char** arrOfNames = (char*)malloc(sizeof(int) * sizeOfMemory);` No, it does not make the slightest sense. Why do you have a pointer to pointer to character but allocate size of int? If you meant to allocate a chunk of raw data, then you'd use a single pointer, not a pointer to pointer. — Lundin, Apr 30 '21 at 08:27
You forgot `#include `. This can have fatal consequences depending on your platform. Didn't you get compiler warnings? — Jabberwocky, Apr 30 '21 at 08:27
[Do I cast the result of malloc?](https://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc/605858) The answer is ***no***. So you have *many* problem in the code you show, all leading to *undefined behavior*. — Some programmer dude, Apr 30 '21 at 08:28

score 1 · Accepted Answer · edited May 11 '21 at 05:23

First of all remove the unnecessary (and partly wrong) casts of the return value of malloc and realloc. In other words: replace (char*)malloc(... with malloc(..., and the same for realloc.

Then there is a big problem here: realloc(arrOfNames, sizeof(int) * sizeOfMemory) : you want to allocate an array of pointers not an array of int and the size of a pointer may or may not be the same as the size of an int. You need sizeof(char**) or rather the less error prone sizeof(*arrOfNames) here.

Furthermore this in too convoluted (but not actually wrong):

*(arrOfNames[i]) = '\0';
strncat(arrOfNames[i], name, strlen(name) + 1);

instead you can simply use this:

strcpy(arrOfNames[i], name);

Same thing in the sort function.

Keep your code simple.

But actually there are more problems in your sort function. You naively swap the contents of the strings (which by the way is inefficient), but the real problem is that if you copy a longer string, say "Walter" into a shorter one, say "Joe", you'll write beyond the end of the allocated memory for "Joe".

Instead of swapping the content of the strings just swap the pointers.

I suggest you take a pencil and a piece of paper and draw the pointers and the memory they point to.

HEAP CORRUPTION DETECTED: after normal block(#87)

1 Answers1