i dont normally work in the console/terminal window in my day job. so, i keep rewriting this every time i respond to someone needing help related to getting user input. im putting it here partly so i dont have to keep rewriting it.

this function will get a string input safely from the user and flush any extra characters if they enter too many.

if you (the programmer) need to get an integer or double floating point value from your program's user, then just follow up this code with a call to the strtol() or strtod() function from the standard C library, <stdlib.c>

this function forces the user who inputs the string to stay within boundaries, but just as importantly, it does not tempt the programmer (you) to use dangerous functions that could overflow buffers or get input "trapped", or to use undefined functions to try and flush the input buffer, etc.

And if your user does not input the correctly formatted value(s) that your program is looking for, strtol, strtod, etc. will point that out and allow you to handled invalid input gracefully.

Edited 6 Years Ago by jephthah: n/a

Comments
I commend you for posting
//*****************************************************************************************//
// GetUserInput()
//
// (c) 2010 by Jephthah and distributed for general use under the WTFPL licence
//
// purpose:gets user input, removes the newline, passes back results up to maximum
//         number of characters, flushes remaining characters from input buffer,
//         returns the number of total characters entered by user
//
// input:  maxStringLength is the maximum allowble characters to input by user
//
// output: returnStr, contains up to the maximum length of characters allowed that were
//         input by the user, any additional characters entered are lost
//
// return: total characters entered by user; caller should check that this value is equal
//         to or less than the maximum allowed to indicate valid string was input.  larger
//         value returned than was allowed by the input indicates characters were lost
//
//
// TYPICAL USE EXAMPLE:
//
//    do {
//       printf("\nEnter String : ");
//       fflush(stdout);
//       inputLen = getUserInput(inputStr, MAX_STR_LEN);
//       if (inputLen > MAX_STR_LEN)
//           printf("string length %d, max length is %d, try again.\n\n",inputLen, MAX_STR_LEN);
//    }
//    while (inputLen > MAX_STR_LEN);
//
//*****************************************************************************************//
int getUserInput (char * returnStr, int maxStringLength)
{
   char    *tempStr;
   int     maxLen, totalCount = 0;
   size_t  len;

   maxLen = maxStringLength + 2;     //account for NULL and /newline
   tempStr = malloc(maxLen * sizeof(char));  //temporary holder

   do {
      fgets(tempStr, maxLen, stdin);  // get chars from input buffer
      len = strlen(tempStr);

      if (tempStr[len-1] == '\n') // newline indicates end of string
      {
         tempStr[len-1] = '\0';   // delete it
         len = strlen(tempStr);   // and recalc length
      }
      totalCount += (int)len;
   }
   while ((int)len > maxStringLength);  // continue to flush extras if too long

   strcpy(returnStr,tempStr);  // copy temp string into output
   free(tempStr);              // and release memory

   return totalCount;   // may be more than the number allowed
}

Good notion, but it doesn't work. I input "Now is the time for all good men to come to the aid of their country." and all I got back was "untry". I was expecting to get back the first 20 characters of what I had typed, and your function to toss the rest of the characters into the bit bin.

int main()
{
    char buf[20] = {0};
    int x = getUserInput (buf, sizeof(buf));
    printf("x = %d\n%s\n", x,buf);
}

Now is the time for all good men to come to the aid of their country.
x = 69
untry.
Press any key to continue . . .

Edited 6 Years Ago by Ancient Dragon: n/a

Oh, damn. you're right, it is broken.

my original was working by having the first fgets outside of the while loop with a second fgets inside. at the last minute, I condensed it all into a single do/while loop to make it more concise ... and neglected to perform a basic challenge to check my code

:icon_redface:

moving the strcpy to inside the do/while loop as a conditional fixes it, so it performs correctly.

thanks for checking it out and letting me know. how embarrassing.

here is the fixed version:

#include <stdio.h>
#include <stdlib.h>

//*****************************************************************************************//
// GetUserInput()
//
// (c) 2010 by Jephthah and distributed for general use under the WTFPL licence
//
// purpose:gets user input, removes the newline, passes back results up to maximum
//         number of characters, flushes remaining characters from input buffer,
//         returns the number of total characters entered by user
//
// input:  maxStringLength is the maximum allowble characters to input by user
//
// output: returnStr, contains up to the maximum length of characters allowed that were
//         input by the user, any additional characters entered are lost
//
// return: total characters entered by user; caller should check that this value is equal
//         to or less than the maximum allowed to indicate valid string was input.  larger
//         value returned than was allowed by the input indicates characters were lost
//
//*****************************************************************************************//
int getUserInput (char * returnStr, int maxStringLength)
{
   char    *tempStr;
   int     totalCount = 0, overflow = 0;
   size_t  len;

   tempStr = malloc((maxStringLength+2) * sizeof(char));  //temporary holder

   do {
      fgets(tempStr, maxStringLength+1, stdin);  // get chars from input buffer
      len = strlen(tempStr);

      if (tempStr[len-1] == '\n') // newline indicates end of string
      {
         tempStr[len-1] = '\0';   // delete it
         len = strlen(tempStr);   // and recalc length
      }

      if (!overflow++)
         strcpy(returnStr,tempStr);  // copy first string into output

      totalCount += (int)len;
   }
   while ((int)len == maxStringLength);  // continue to flush extras if too long

   free(tempStr);              // and release memory

   return totalCount;   // may be more than the number allowed
}

int main()
{
   char buf[20] = {0};
   int x;
   while(1)
   {
      printf("Enter String: ");
      fflush(stdout);
      x = getUserInput (buf, sizeof(buf));
      printf("Entered %d Chars Total.\nSaved Result: %s\n\n", x, buf);
   }
   return 0;

}

@Jephthah: (In your fixed snippet)
You can replace line 38 ( len = strlen(tempStr); ) by len--; :)

Edited 6 Years Ago by mvmalderen: n/a

true. it must've made sense at one time why i did it the long way but, a few edits later, i can't recall what it was.

:)

strcpy() and strlen() requires the header file string.h included.
malloc()'s return needs to be checked before using the memory.

jephthah>

// return: total characters entered by user; caller should check that this value is equal
// to or less than the maximum allowed to indicate valid string was input. larger
// value returned than was allowed by the input indicates characters were lost

I find the return of the string or null more useful that the total amount of characters.

char *getUserInput (char * returnStr, int maxStringLength)

With such I could even:

printf("The text entered was %s\n", getUserInput( buf, sizeof(buf)));

Now, instead of using fgets(), malloc(), strcpy(), strlen(), the same result could be achieved with just getchar() and a couple ifs:

#include <stdio.h>

size_t obtainput (char *orig, size_t size)
{
    size_t i = 0;
    size_t  total = 0;

    --size;
    for (;;) {
        int chr = getchar();
        if (chr == '\n' || chr == EOF) break;
        if (i < size) orig[i++] = chr;

        ++total;
    }
    orig[i] = '\0';
    return total;
}

Edited 6 Years Ago by Aia: n/a

Comments
your way does seem better. concise is better.
Solid advice :)

In line 29: [B]sizeof(char)[/B] is - by definition - equal to 1.
So this: tempStr = malloc((maxStringLength+2) * [B]sizeof(char)[/B]); .
Could be rewritten as: tempStr = malloc(maxStringLength+2) .
However leaving the sizeof out won't increase performance, since it is evaluated at compile-time, it's just a matter of personal preference.

Edited 6 Years Ago by mvmalderen: n/a

Since this thread's title is "How to Get User Input from console -- safely.", I think it's appropriate to point out a little thing. Namely, in the Aia's version, decrementing size before entering the for-loop is a recipe for buffer overflow if size 's initial value (for whatever reason) is zero.
So, to be on safer side ..

size_t obtainput (char *orig, size_t size)
{
    size_t i = 0;
    size_t  total = 0;
	
    /* Safety check ... */
    if(size == 0)
        return size;

    --size;
...
Comments
Excellent catch!

Both versions of getUserInput will put the null terminator just past the end of returnStr, in returnStr[maxStringLength], if at least maxStringLength-1 characters are read (and there is, as a result, no \n to replace with \0). Try watching, with a debugger, what happens to returnStr when you type at least maxStringLength-1 characters.

This is assuming that maxStringLength == sizeof(returnStr), but that's the way you used it, and the way I would expect it to work.

I think you can fix it just by always setting tempStr[maxStringLength-1] to \0 before you copy it into returnStr. Even if there's already a \0 closer to the beginning of tempStr, it wouldn't cause any problems.

The article starter has earned a lot of community kudos, and such articles offer a bounty for quality replies.