Search code examples
cpointersdynamic-memory-allocationrealloc

Segmentation fault every third running my program, using mallloc


I am getting a segmentation fault almost every third time, and I am trying to understand why. I think the reason for it is using malloc() and free() wrong. I have to read the user stdin and then save it in an array using malloc. This part was working, until somehow the errors started happening.

My code:

char *Input() {
    char user_input;
    int length;

    char *buffer = malloc(2 * sizeof(char));

    while (((user_input = getchar(stdin)) != EOF) && (user_input != '\n')) {
        buffer[length] = user_input;
        length++;
        char *buffer_new = realloc(buffer, length + 2);
        if (buffer_new != NULL) {
            buffer = buffer_new;
        } else {
            free(buffer);
            printf("Error.\n");
            return 1;
        }
    }

    buffer[length] = '\0'; 

    if (strlen(buffer) > 200) {
        printf("Error.\n");
        return 2;
    }

    return buffer;
}

I am not really sure where my mistake is or why the error happens. Here is how I call the function:

int main() {
    char *input = Input();

    if (input == 1)
        return 1;
    if (input == 2)
        return 2;

    free(*input);

    return 0;
}

Solution

  • The primary problem I see is, in your code for Input() function, length is a local variable with automatic storage duration and not initialized explicitly. So, it contains indeterminate value. Thus, the statement

     buffer[length] = user_input;
    

    is accessing invalid memory address. This invokes undefined behavior.

    You have to explicitly initialize the length to 0.

    After that, know that getchar() returns an int, and a value like EOF cannot fit into a char. You need to change the user_input to int.

    Also, in the main(), you have defined input as a pointer,

    char* input = Input();
    

    but, you are comparing it against an int value, this is most likely not what you want.

    You should be comparing against the value stored in the pointer, something like

    if (*input == 1)
        return 1;
    if (*input == 2)
        return 2;
    

    and finally, you pass the pointer to free(), not the content, so change

     free(*input);
    

    to

     free(input);