Search code examples
cvalidationscanfuser-input

How to handle buffer error while validating user input


I need to read in user input as an integer to pass it to my other function. If I use my validation (code below), it crashes after 4 bad inputs. I'm not completely sure if this is even a buffer error or not. But I also didn't find a proper way to validate my input and handle the errors. I didn't use scanf(%d) on purpose because I wanted to dodge the warning CLion is giving me when using it. I hope someone here can explain to me why my code is crashing after 4 bad inputs and how to fix it, or show me an alternative way.

char *userInput = malloc(100);
long amountOfPlayers;
//Todo: More Validation needed, bufferoverflow
for (int i = 0; i < sizeof(userInput) / sizeof(*userInput); i++) {
    char *end;
    printf("Please enter the amount of players: ");
    scanf("%s", userInput);
    amountOfPlayers = strtol(userInput, &end, 10);
    if (end == userInput) {
        printf("wasn't a number\n");
    }
    else if (end[0] != '\0') {
        printf("trailing characters after number %ld: %s\n", amountOfPlayers, end);
    }
    else
        return init_playerList(amountOfPlayers);
}

Solution

  • userInput is a pointer, not an array, so sizeof(userInput) returns the size of a pointer, typically 4 bytes. sizeof(*userInput) is sizeof(char), which is 1. So sizeof(userInput) / sizeof(*userInput) is 4, which means your for loop only executes 4 times. See How to find the 'sizeof' (a pointer pointing to an array)?

    There's no need for a for loop, just use while (true). You're not doing anything that iterates over the elements of userInput, it's just the buffer.

    There's also no reason to allocate it with malloc(), you can simply declare:

    char userInput[100];
    

    You have a memory leak because you never free(userInput) before returning from the function. But if you declare it as an array this is not necessary.

    TO prevent buffer overflow you should use:

    scanf("%100s", userInput);