How to handle buffer error while validating user input

Question

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

`sizeof(userInput)` is the size of a pointer, not the size of the array. — Barmar, Feb 02 '18 at 23:17
Why are you using the size of `userInput` as the limit of the loop, anyway? — Barmar, Feb 02 '18 at 23:19
Man you solved the problem, i wanted to answer that the for loop isnt doing a lot of work here and i guess it can be changed out for a simple while(true), for fun i did it and now its working. Can you maybe explain me the mistake i did there ? — XaNNy0, Feb 02 '18 at 23:22

Barmar · Accepted Answer · 2018-02-02T23:27:28.447

1

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

edited Feb 02 '18 at 23:27

answered Feb 02 '18 at 23:24

Barmar

741,623
53
500
612

doesnt a pointer equals a one dimensional array ? char *a = malloc(100) same as char a[100] ? – XaNNy0 Feb 02 '18 at 23:26
Arrays decay to pointers in many contexts, but pointers never become arrays. – Barmar Feb 02 '18 at 23:28
thanks a lot barmar, ill take a look on your link, finally i can go to bed – XaNNy0 Feb 02 '18 at 23:28

score 0 · Answer 2 · answered Feb 02 '18 at 23:23

sizeof(userInput) / sizeof(*userInput) won't return the number of elements, because userInput is a pointer, not an array. This only works for pure arrays. In case of pointer is always return the same value: size of a pointer divided by the size of the object.

int size = 100;
char *userInput = malloc(size);
if(userInput == NULL)
{
    // error handling
}

for (int i = 0; i < n; i++) {
    ....
}

would be correct.

How to handle buffer error while validating user input

2 Answers2