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 . . .
Ancient Dragon
Retired & Loving It
30,049 posts since Aug 2005
Reputation Points: 5,662
Solved Threads: 2,343
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
Posting Maven
2,587 posts since Feb 2008
Reputation Points: 2,143
Solved Threads: 179
@Jephthah: (In your fixed snippet)
You can replace line 38 ( len = strlen(tempStr); ) by len--; :)
tux4life
Nearly a Posting Maven
2,350 posts since Feb 2009
Reputation Points: 2,134
Solved Threads: 243
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.
:)
jephthah
Posting Maven
2,587 posts since Feb 2008
Reputation Points: 2,143
Solved Threads: 179
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;
}
Aia
Nearly a Posting Maven
2,392 posts since Dec 2006
Reputation Points: 2,224
Solved Threads: 218
In line 29:
<strong>sizeof(char)</strong> is - by definition - equal to1.
So this:
tempStr = malloc((maxStringLength+2) * <strong>sizeof(char)</strong>); .
Could be rewritten as: tempStr = malloc(maxStringLength+2) .
However leaving thesizeof out won't increase performance, since it is evaluated at compile-time, it's just a matter of personal preference.
tux4life
Nearly a Posting Maven
2,350 posts since Feb 2009
Reputation Points: 2,134
Solved Threads: 243
tux4life
Nearly a Posting Maven
2,350 posts since Feb 2009
Reputation Points: 2,134
Solved Threads: 243
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;
...
mitrmkar
Posting Virtuoso
1,809 posts since Nov 2007
Reputation Points: 1,105
Solved Threads: 395
