Thread: fgets prompt in while loop "skips"

  1. #1
    Registered User
    Join Date
    Aug 2008
    Posts
    129

    fgets prompt in while loop "skips"

    I was just testing the CLI for my application, and I noticed that fgets isn't waiting for a response all the time. It's a good bit more complex than this, but here's a patchwork example:
    Code:
    void start(struct outline_interface *this){
    	unsigned short length = 31;
    	char *command = (char *)NULL, *arguments = (char *)NULL;
    	
    	char *space;
    	unsigned short valid;
    	
    	while(1){
    		printf("Enter a command. > ")
    		fgets(command, (int)length, stdin);
    		/* [putting everything after the first space out of 'command' and into 'arguments'] */
    		
    		if(strcmp("make", command) == 0){
    			char response[4];
    			printf("No outline with this name is currently in memory; do you want to load it? (yes, no)");
    			fgets(response, 4, stdin);
    			if(strcmp(response, "yes") == 0)
    				this->load(this, arguments); /* performs no user I/O */
    			printf("These messages may be avoided by using the otherwise identical \"load\" command to create/load outlines.");
    		}
    		/* [checking for exit conditions] */
    	}
    }
    When I run it, I reply to two prompts and then one "skips," taking an empty string as input:
    Code:
    Enter a command. > make Test
    No outline with this name is currently in memory; do you want to load it? (no, yes) > yes
    These messages may be avoided by using the otherwise identical "load" command to create/load outlines.
    [formatted user input]
    Enter a command. > Invalid input.  Type "help" for a list of accepted commands.
    Enter a command. >
    What could cause that? Do I need to system("pause")?

    Thanks!
    Last edited by Jesdisciple; 04-12-2009 at 03:33 PM. Reason: accidental submission

  2. #2
    Registered User
    Join Date
    Aug 2008
    Posts
    129
    Alright, I got the text in my post as it should be...

  3. #3
    Registered User
    Join Date
    Sep 2004
    Location
    California
    Posts
    3,268
    Check the return value of fgets(). It could be returning NULL, but you are processing the buffer anyways.

  4. #4
    Registered User
    Join Date
    Sep 2004
    Location
    California
    Posts
    3,268
    Code:
    fgets(response, 4, stdin);
    		if(strcmp(response, "yes") == 0)
    			this->load(this, arguments); /* performs no I/O */
    Also, keep in mind that this code will leave the '\n' character in the input buffer. That means the next time you call fgets(), you will just receive an empty string with a newline character in it.

  5. #5
    Registered User
    Join Date
    Sep 2007
    Posts
    1,012
    fgets() only reads as much as it can, and one reason it stops reading is that the limit you pass as the second argument is reached. So you have this:
    Code:
    fgets(response, 4, stdin);
    This means fgets() will read at most three characters, with the fourth byte being used only to store the null character. So you type in "yes" and hit enter. fgets() reads the "yes", and since that's the limit, it stores those four characters in "response"; but you hit enter after typing "yes", so there's still a newline that fgets() ignored. The next call to fgets() sees that newline and reads only that (being that fgets() was designed to stop reading when it sees a newline). Thus it appears as though fgets() is being skipped, but it's not; it's just reading the rest of the input.

    This is an issue regardless of whether you limit the buffer to 4 bytes or 4000; the user can always type more. The quick and dirty way to fix the problem is to make your response buffer a lot larger and just ignore any junk the user types in (or do with it what you will). That's somewhat of a band-aid, but there's always going to be some issue or other when you're reading input in C.

    Incidentally, the casts you are using are not necessary. While they don't break the code, casts are usually used as a way to get around C's type system, and not to reaffirm valid code. After all, you aren't writing:
    Code:
    unsigned short length = (unsigned short)31;
    and that's really no different than:
    Code:
    char *command = (char *)NULL;
    31 (an int) can be converted to an unsigned short just as well as NULL can be converted to a char *. No need to try to help C do it.

  6. #6
    Registered User
    Join Date
    Dec 2008
    Posts
    104
    Quote Originally Posted by cas View Post

    This is an issue regardless of whether you limit the buffer to 4 bytes or 4000; the user can always type more. The quick and dirty way to fix the problem is to make your response buffer a lot larger and just ignore any junk the user types in (or do with it what you will). That's somewhat of a band-aid, but there's always going to be some issue or other when you're reading input in C.
    You could always, of course, use scanf(), which limits depend on the variable you are using as a buffer, rather than having a pre-destined limit.

  7. #7
    Registered User
    Join Date
    Aug 2008
    Posts
    129
    Oh... As I was reading I first thought the 4 was inaccurate but looking at my code it's not. Here's the code which the 4 took the place of, with the intent of finding how much I should ask fgets for:
    Code:
    	/* Find the size of the largest option. */
    	for(i = 0; i < length; i++)
    		size = MAX(size, strlen(items[i]));
    	size++;
    I was thinking I could just make it size += 2, but do I really need to worry about some goof typing "lihsgoiwrbghv"?

  8. #8
    Registered User
    Join Date
    Sep 2004
    Location
    California
    Posts
    3,268
    but do I really need to worry about some goof typing "lihsgoiwrbghv"?
    That's up to you. How robust do you want your application to be?

  9. #9
    Registered User
    Join Date
    Aug 2008
    Posts
    129
    I apparently need a bit of enlightenment... fgets prevents buffer overflows for me, and I've allowed enough space for all valid input. How is treating nonsense as sensible robust?

    EDIT: Oh, and re casting: Looking back at the 3rd C commandment, I find that I remembered it incorrectly.
    Last edited by Jesdisciple; 04-12-2009 at 04:39 PM.

  10. #10
    Registered User
    Join Date
    Sep 2004
    Location
    California
    Posts
    3,268
    How is treating nonsense as sensible robust?
    That's not what I said or implied. Being able to handle nonsense input without disrupting your application is robust. If your buffer size is the max expected input size, that's fine. But not accounting for the fact that a user can input more characters than this expected size is not robust. If your call to fgets() does not end with a newline character, then you should read from the input buffer until a newline character is found.
    Code:
    while ((ch = getchar()) != '\n' && ch != EOF);

  11. #11
    Registered User
    Join Date
    Aug 2008
    Posts
    129
    Yeah, that was a sloppy question, so I apologize. Preparation isn't exactly treatment, but the point I was trying to get across is that it doesn't matter much if someone abuses a program and sees it hiccup. Or maybe it does and I need enlightenment...

    But would the given code expect another character from the user if no newline were still in the buffer?

    Thanks for the help.

  12. #12
    Registered User
    Join Date
    Sep 2007
    Posts
    1,012
    Quote Originally Posted by abraham2119 View Post
    You could always, of course, use scanf(), which limits depend on the variable you are using as a buffer, rather than having a pre-destined limit.
    I'm not sure I understand your statement, but scanf() is not better than fgets() in this regard. Either you give no size to scanf() and it can overflow, or you give it a size and it acts similar to fgets(). There's no standard C function that stores an unknown amount of data. You can look to something like GNU's getline() (among many others) for "unlimited" data input, but then, you still are limited by memory.

  13. #13
    Registered User
    Join Date
    Sep 2004
    Location
    California
    Posts
    3,268
    But would the given code expect another character from the user if no newline were still in the buffer?
    That's why I said "If your call to fgets() does not end with a newline character..."

    Input from the user will always end with a newline character. If your call to fgets() doesn't return a newline character as the last byte (aside from the null terminator), then there is still a newline character in the input buffer.

  14. #14
    Registered User
    Join Date
    Aug 2008
    Posts
    129
    Alright, I reverted the other bugfix to test this one, but the output is the same as it was at the beginning... Is my condition giving a bad "false" result?
    Code:
    	fgets(input, (int)length, stdin);
    	if(strlen(input) == length && input[length - 1] != '\n'){
    		while ((ch = getchar()) != '\n' && ch != EOF)
    			/* Clean out the buffer. */;
    	}
    EDIT: Never mind; I needed to subtract 1 from length both times. Thanks again!
    Last edited by Jesdisciple; 04-12-2009 at 05:40 PM.

  15. #15
    Registered User
    Join Date
    Sep 2007
    Posts
    1,012
    Quote Originally Posted by Jesdisciple View Post
    EDIT: Oh, and re casting: Looking back at the 3rd C commandment, I find that I remembered it incorrectly.
    While a lot of the commandments still apply, they originate from pre-ANSI days (the version here makes that pretty clear); consequently, the third commandment is, for the most part, obsolete.

    As it is stated at the above URL, "Thou shalt cast all function arguments to the expected type if they are not of that type already, even when thou art convinced that this is unnecessary, lest they take cruel vengeance upon thee when thou least expect it." This made sense back before we all had ANSI compilers, because C worked like this:
    Code:
    #include <stdio.h>
    void f();
    int main(void)
    {
      f(5);
      f(5LL);
    }
    void f(v)
    unsigned long long v;
    {
      printf("%lld\n", v);
    }
    OK, I'm mixing C99 (void return type, long long) and K&R C, which is an abomination, but I'm doing it for the sake of illustration.

    Note the declaration for the function f(): it uses an empty set of parentheses, instead of (long long). The empty parentheses mean "unspecified arguments", so C has no idea how the function is set up; that's up to you to get right. Pre-ANSI C had no prototypes.

    What happens, then, is that (after something called the default promotions), the values are passed as the types that they are. This means in the f(5) call above, 5 is passed as an int. This is, of course, wrong, because f() takes a long long; but the compiler can't know that. So an int is passed where a long long is expected. With gcc I get 577750908939534341 printed out; with clang I get -5188481073805262843; with klcc I get -4636931200538116091; and with Intel's compiler I get 5. What do you know, sometimes broken code gives the right value.

    The second call, f(5LL), is correct (it's the same as doing f( (long long)5 ), but nicer looking). This is because 5LL is a long long, which gets passed as a long long, and so is the proper type.

    On the issue of NULL, the commandments linked above say: "Also, contrary to the beliefs common among the more backward inhabitants of the Polluted Eastern Marshes, `NULL' does not have a pointer type, and must be cast to the correct type whenever it is used as a function argument."

    The same applies here. NULL in ANSI C is a null pointer constant, and everywhere a pointer is expected, it provides a null pointer. You can pass it to a function expecting a char*, and it will be passed as a char* (assuming you provided a prototype, of course, and there is no reason not to).

    Basically, prototypes essentially render the third commandment worthless. Prototypes are great and should always be used.

    There is one caveat, though! Some C functions (such as printf() ) can take a variable number of arguments. For such variable arguments, the K&R semantics apply! Therefore, even though you have a prototype for printf(), be careful:
    Code:
    printf("%lld\n", 5); /* BAD because 5 is not a long long */
    printf("%lld\n", 5LL); /* OK */
    printf("%p\n", NULL); /* BAD because NULL is not necessarily a void* */
    printf("%p\n", (void *)NULL); /* OK */
    Apart from this little quirk, prototypes mean that arguments are properly converted before functions are called.

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Can a "switch" be inside a loop?
    By gmk0351 in forum C Programming
    Replies: 5
    Last Post: 03-28-2008, 05:47 PM
  2. need help, fgets won't stop while loop
    By Enkid in forum C Programming
    Replies: 6
    Last Post: 10-26-2007, 07:15 AM
  3. loop needed also how to make input use letters
    By LoRdHSV1991 in forum C Programming
    Replies: 3
    Last Post: 01-13-2006, 05:39 AM
  4. when a while loop will stop ?
    By blue_gene in forum C Programming
    Replies: 13
    Last Post: 04-20-2004, 03:45 PM
  5. need to loop my prog. after the prompt
    By d-dubb in forum C++ Programming
    Replies: 2
    Last Post: 10-10-2002, 01:39 PM