realloc fails to reallocate a previously malloc-ed pointer

Question

I'm working on a function to read a full line from stdin into a char* using getchar(), and it mostly works, but when I input a longer string I get

realloc(): invalid next size: 0x00000000007ca010

Here's the function:

char *readCMDLine() {
    char *cmdline = malloc(sizeof(char));
    int counter = 0;
    char c;
    while (c != 10) {
        c = getchar();
        cmdline = realloc(cmdline, sizeof(cmdline) + sizeof(char));
        cmdline[counter] = c; 
        counter++;
    }
    return cmdline;
}

Is there a way to fix this so that I could read larger lines with it?

Answer 1

There are multiple problems with your code:

• You test while (c != 10), but c is not initialized the first time and you do not allow for the file to end before the next '\n'. You also assume ASCII, some systems still use EBCDIC today, where '\n' is not 10.
• You realloc using sizeof(cmdline) instead of counter. sizeof(cmdline) is the size of the pointer, not the currently allocated array size.
• You store getchar() into a char variable, EOF is not a value that can fit there, nor possibly values between 128 and UCHAR_MAX if char is signed on your platform. Use int for this.
• You do not null terminate the allocated string. Neither malloc not realloc initialize the allocated memory.
• You never check for malloc or realloc failure.
• You should probably return NULL upon end of file, otherwise there is no way to tell end of file from sequences of empty lines.
• You reallocate the buffer for each character, this is inefficient, but can be optimized at a later stage, fix the code first.

Here is a corrected version:

#include <stdlib.h>

char *readCMDLine(void) {
    char *cmdline = malloc(1);
    size_t counter = 0, size = 1;
    int c;

    if (cmdline == NULL)
        return NULL;

    while ((c = getchar()) != EOF && c != '\n') {
        char *p = realloc(cmdline, counter + 2);
        if (p == NULL) {
            free(cmdline);
            return NULL;
        }
        cmdline = p;
        cmdline[counter++] = c;
    }
    cmdline[counter] = '\0';
    if (c == EOF && counter == 0) {
        free(cmdline);
        cmdline = NULL;
    }
    return cmdline;
}

Here is an optimized version that calls realloc much less:

#include <stdlib.h>

#define ALLOCATE_INCREMENT  100
char *readCMDLine(void) {
    char *p, *cmdline = NULL;
    size_t counter = 0, size = 0;
    int c;

    while ((c = getchar()) != EOF && c != '\n') {
        if (counter > size - 2) {
            p = realloc(cmdline, size += ALLOCATE_INCREMENT);
            if (p == NULL) {
                free(cmdline);
                return NULL;
            }
            cmdline = p;
        }
        cmdline[counter++] = c;
    }
    if (counter == 0) {
        if (c == EOF || (cmdline = malloc(1)) == NULL)
            return NULL;
    } else {
        p = realloc(cmdline, counter + 1);
        if (p != NULL)
            cmdline = p;
    }
    cmdline[counter] = '\0';
    return cmdline;
}