strcmp not working

Question

I know this may be a totally newbie question (I haven't touched C in a long while), but can someone tell me why this isn't working?

printf("Enter command: ");
bzero(buffer,256);
fgets(buffer,255,stdin);

if (strcmp(buffer, "exit") == 0)
    return 0;

If I enter "exit" it doesn't enter the if, does it have to do with the length of "buffer"?

Any suggestions?

Analcite

Answer 1

You want to do this:

strcmp(buffer, "exit\n")

That is, when you enter your string and press "enter", the newline becomes a part of buffer.

Alternately, use strncmp(), which only compares n characters of the string

Jarid

That's good unless the user types a space before, or after, exit. And don't forget the (probably apocryphal) story of the system that stopped working when Ecuador was added - people had to type Quito for the name of the capital, and the program exited (quit) because only the first 4 letters were compared with 'quit'. Quite nasty! – Montemontefiascone

@Jonathan truths! my own suggestion is to ensure that the user is never allowed to input anything into your program. – Jarid

Answer 2

fgets() is returning the string "exit\n" -- unlike gets(), it preserves newlines.

Bunkmate

... and "gets" only takes one parameter (buffer in this example). Thanks! – Analcite

(Please don't use gets() in public.) – Bunkmate

@Jian, because gets() doesn't protect you against buffer overruns. – Beauchamp

Answer 3

As others have said, comparing with "exit" is failing because fgets() included the newline in the buffer. One of its guarantees is that the buffer will end with a newline, unless the entered line is too long for the buffer, in which case it does not end with a newline. fgets() also guarantee that the buffer is nul terminated, so you don't need to zero 256 bytes but only let fgets() use 255 to get that guarantee.

The easy answer of comparing to exactly "exit\n" required that the user did not accidentally add whitespace before or after the word. That may not matter if you want to force the user to be careful with the exit command, but might be a source of user annoyance in general.

Using strncmp() potentially allows "exited", "exit42", and more to match where you might not want them. That might work against you, especially if some valid commands are prefix strings of other valid commands.

In the general case, it is often a good idea to separate I/O, tokenization, parsing, and action into their own phases.

Ennis

Use: fgets(buffer, sizeof(buffer), fp); and don't subtract one (or use 255) because fgets() behaves sanely - you told it how much space there is, and it ensures it uses no more, placing the null terminator in the last available character in the array. – Montemontefiascone

Exactly what I was trying to say... but better said. ;-) – Ennis

Answer 4

Agree with Dave. Also you may wish to use strncmp() instead. Then you can set a length for the comparison.

http://www.cplusplus.com/reference/clibrary/cstdio/fgets/

http://www.cplusplus.com/reference/clibrary/cstring/strncmp/

Quentinquercetin

Answer 5

I'd recommend that you strip the \n from the end of the string, like this.

char buf[256];
int len;
/* get the string, being sure to leave room for a null byte */
if ( fgets(buf,sizeof(buf) - 1) == EOF )
{
  printf("error\n");
  exit(1);
}
/* absolutely always null-terminate, the easy way */
buf[sizeof(buf) - 1] = '\0';
/* compute the length, and truncate the \n if any */
len = strlen(buf);
while ( len > 0 && buf[len - 1] == '\n' )
{
  buf[len - 1] = '\0';
  --len;
}

That way, if you have to compare the inputted string against several constants, you're not having to add the \n to all of them.

Kathie

Why the downvote? It is a valid solution though it is not explained. – Euchology