double free or corruption error happens when call free in c

Question

I'm a newer in c, for learning it, i'm trying to write a function to manually read characters from std input. The program will read lines from std and output them, ant it will end when meets an empty line.

But it works well if the input stream only contains three lines or lesser, but it always stopped with an error if the input contains 4+ lines. The error happens when call realloc and free function: 'double free or corruption (fasttop): 0x0000000001f46030 *', why?

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

char *readline(int *length) {
    char ch, *s = NULL, *temp = NULL;
    int UNIT = 3, size = 0, index = 0;
    while ((ch = getchar()) != EOF) {
        if (size == 0 || index >= size) {
            size += UNIT;
            temp = realloc(s, sizeof(char) * size);
            if (s != NULL && temp != s) free(s);
            s = temp;
            temp = NULL;
        }

        s[index++] = (ch == '\n') ? '\0' : ch;
        if (ch == '\n') break;
    }
    *length = index - 1;
    return s;
}

char **readlines(int *count) {
    char **lines = NULL, **tempLines = NULL;
    int UNIT = 1, size = 0, index = 0;
    int length = 0;
    char *line = NULL;
    while ((line = readline(&length)) != NULL) {
        if (strlen(line) == 0) break;
        if (size == 0 || index >= size) {
            size += UNIT;
            tempLines = realloc(lines, size * sizeof(char *));
            if (lines != NULL && tempLines != lines) free(lines);
            lines = tempLines;
            tempLines = NULL;
        }
        lines[index++] = line;
    }
    *count = index;
    return lines;
}

int main(int argc, char *argv[]) {
    int length = 0, index = 0;
    char **lines = readlines(&length);
    printf("The lines you typed are: \n");
    for (; index < length; index++) {
        printf("%5s %s.\n", "-", lines[index]);
    }
    return 0;
}

The execute result is:

xxx@xxx:~/vmshared$ ./mylib2
abc
def
hij

The lines you typed are: 
    - abc.
    - def.
    - hij.
xxx@xxx:~/vmshared$ ./mylib2
11
22
33
44
*** Error in `./mylib2': double free or corruption (fasttop): 0x00000000017f1030 ***

Answer 1

your problem is here:

 temp = realloc(s, sizeof(char) * size);
 if (s != NULL && temp != s) free(s);

in case realloc succeeded, you free s after realloc already freed it. you can see this answer for more details.

Answer 2

there's a problem with your readlines and readline function. your error is caused by freeing a pointer after realloc call.

tempLines = realloc(lines, size * sizeof(char *));
if (lines != NULL && tempLines != lines) free(lines);  // wrong

temp = realloc(s, sizeof(char) * size);
if (s != NULL && temp != s) free(s);  //wrong

if memory content was moved to another location, realloc frees the old pointer for you.
in your main function you never free your lines pointer.

Answer 3

Because you are freeing your data and then using it:

        temp = realloc(s, sizeof(char) * size);
        if (s != NULL && temp != s) free(s);

which then means that you are writing to freed memory - which is bad.

The function realloc can be seen as doing:

void *realloc(void *ptr, size_t new_size)
{
    void* newptr = malloc(size);
    size_t oldsize = find_size(ptr);
    memcpy(newptr, ptr, oldsize);
    free(ptr);
    return newptr;
}

Of course, the REAL realloc is a lot more complex (because it checks the current block to see if it can be expanded before it allocates new data), and probably doesn't call regular malloc, but the functionality is roughly this.

The reason for storing the result of realloc in a different variable than the old pointer is for the case where it returns NULL - it couldn't expand to the new size - at that point, you need a temp and the original pointer, so you don't leak the old pointer's memory.

Answer 4

You should not free the original memory area after a successful call to realloc.

temp = realloc(s, sizeof(char) * size);
if (s != NULL && temp != s) free(s); // This is wrong!

If realloc moves your data, it will also free the old area. You don't need to do that yourself.

Answer 5

When you call realloc() and it is successful, the old memory location has already been freed and the new location is returned. There is a chance that the old and new locations are the same. However, either way, it is incorrect to release the old pointer. It would be eccentric to immediately release the new pointer.

Thus, this code is incorrect:

temp = realloc(s, sizeof(char) * size);
if (s != NULL && temp != s)
    free(s);
s = temp;
temp = NULL;

It should probably be:

temp = realloc(s, size);
if (temp == NULL)
    …report error and exit function…
s = temp;

There's no need to set temp = NULL; after the assignment, though it does no particular harm beyond marginally (immeasurably) slowing the program down.