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);
}
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);