1

Just messing around with a program I'm currently working on and I stumble upon this. The program is of the client-server type so the client asks the user for commands and the server performs all the executions. If the "exit" command is entered the client will stop. I'm using 

if (strncmp ("exit", command, strlen (command) - 1)) == 0)
{
         //Perform exit code
}

to check if "exit" has been entered. If I enter "a " the program exits as if "exit" has been entered. Not a big deal, just wondering why it's happening.

Thanks.

user2929779
  • 359
  • 5
  • 13
  • 6
    Why are you subtracting 1 from `strlen(command)`? –  Dec 02 '13 at 19:24
  • 3
    If `command` ia "a", then `strlen(command)-1` is zero. You aren't comparing anything. – r3mainer Dec 02 '13 at 19:25
  • @squeamishossifrage: That should be entered as an answer. – Eric Postpischil Dec 02 '13 at 19:25
  • Are you sure that your program exits because that `if` gets entered, and not because of some other code path (e.g. the loop which reads commands terminates on unknown commands or so)? – Frerich Raabe Dec 02 '13 at 19:26
  • If the length of `strlen(command)-1` is 0, the strings will be equal. You probably don't want `e` to match `exit` either after you've fixed the off-by-one issue; you are using the wrong length, and probably the wrong comparator (use `strcmp()`, not `strncmp()`). – Jonathan Leffler Dec 02 '13 at 19:26
  • what happens when strlen(command) > strlen("exit")? doesn't look good! – Scotty Bauer Dec 02 '13 at 19:27
  • Depending on the way you read your input, the space could be trimmed from "a " leading to command being "a". This – user1781290 Dec 02 '13 at 19:27
  • write `strlen(command)` instead – Jekyll Dec 02 '13 at 19:27
  • @ScottyBauer: if `strlen(command) > strlen("exit")`, there is no problem. – Jonathan Leffler Dec 02 '13 at 19:28
  • Or, building on @JonathanLeffler you could just change strlen(command)-1 to 4, which is the length of "exit" not including the null terminator, should you insist on having to use strncmp. – ciphermagi Dec 02 '13 at 19:28
  • There is one `)` too many in the statement `if (strncmp ("exit", command, strlen (command) - 1)) == 0)`. This code won't even compile. – johnsyweb Dec 02 '13 at 19:28
  • @JonahNelson: The trouble with using `4` as the length of `exit` with `strncmp()` is that `exiting` will compare equal to `exit`. The correct comparator is `strcmp()`; `strncmp()` is going to give problems unless the length specified is at least as big as the longer of the two strings, in which case you might as well have used `strcmp()` anyway. – Jonathan Leffler Dec 02 '13 at 19:29
  • Sorry guys, should've specified. I'm using strncmp cause I'm using fgets to get what the user entered. Command would actually contain "exit\n". So strlen (command)-1 as the last parameter lets me compared just "exit". – user2929779 Dec 02 '13 at 19:31
  • Tokenize your input properly so that the string doesn't contain the newline. Or use a comparator that takes two lengths, and returns immediately with not-equal if the lengths are different. As it stands, if the user types `"e\n"`, the length will be 1, and that will match `"exit"` to a length of 1. So `e` is a synonym for `exit`, which is probably not a good idea. – Jonathan Leffler Dec 02 '13 at 19:31
  • @JonathanLeffler Obviously that is a problem. It's all about design considerations and intentions at that point. I don't really think that strncmp() is the right way to go either, which is why I said "should you insist..." Reminds me of an argument I got into once with someone who claimed it was impossible to use strlen(x) as an argument e.g. strncmp(str1, str2, strlen(str2) + 1). It was pretty stupid. In any case, making it 5 would enforce limits, because it would force the character after 't' to be '\0'. – ciphermagi Dec 02 '13 at 19:37
  • @JonathanLeffler Yeah that would be a problem. But entering "a " still shouldn't be a problem though right? n would be 1 so it would compare "a" with "e". – user2929779 Dec 02 '13 at 19:38
  • @JonahNelson strncmp is just easier to implement. That's why I chose it. – user2929779 Dec 02 '13 at 19:39
  • If there's a space after the `a`, then `strncmp("exit", "a ", n);` for any value of `n > 0` should not compare equal. But be very careful when using `strncmp()`. It looks like you've got a plausible solution from @dasblinkenlight; I remember having similar length-testing code in one of my programs, now I've seen his solution, though I think I was using `strcmp()` even so. (If the lengths are known, the length test is simpler than the string comparison, and the lengths were known in the relevant context.) – Jonathan Leffler Dec 02 '13 at 21:05

4 Answers4

1

Just use strcmp.

strlen("a") return 1... 1 minus 1 => zero. The third parameter is zero so the strncmp reached the "max length" without testing any char on both strings.

Fiddling Bits
  • 8,712
  • 3
  • 28
  • 46
Mauro H. Leggieri
  • 1,084
  • 11
  • 25
1

Here is what happens: when you enter "a ", the code that separates the command out makes the command string equal to "a" (no space). Its length is one. Then the code calls strncmp, telling it to compare "exit" using zero initial characters. This comparison always succeeds, because no comparisons are made at all.

Note that removing -1 from the expression may not fix the problem, because the code would then exit when you enter "e " (unless of course that is the behavior that you want to achieve). To make sure that the command is indeed "exit", you can use this condition:

if ( strlen(command) == 4 && strncmp("exit", command, 4)) == 0 ) ...
Sergey Kalinichenko
  • 714,442
  • 84
  • 1,110
  • 1,523
  • We do not know that an “equal” result for “exit” compared to “e” is incorrect; the goal might be to return true if the command is any prefix of “exit”, thus permitting the user to enter abbreviations. `strncmp("exit", command, strlen(command)) == 0` would suit this fine (with caveats about null strings in `command`). – Eric Postpischil Dec 02 '13 at 19:30
  • @EricPostpischil You are right, perhaps OP wants this behavior. I changed the wording to account for that. Thanks! – Sergey Kalinichenko Dec 02 '13 at 19:32
  • @dasblinkenlight Yeah this is exactly what's happening. Thanks a lot. Shoulda debugged a bit more haha – user2929779 Dec 02 '13 at 19:44
  • Since the string length is known, could do memcmp("exit", command, 4) – chux - Reinstate Monica Dec 02 '13 at 19:44
  • 1
    @chux: That returns an indication of equality when `command` is “exiting”, which is probably not what is wanted. – Eric Postpischil Dec 02 '13 at 19:47
  • @Eric Postpischil Appears my suggestion was not clear: `if (strlen(command) == 4 && memcmp("exit", command, 4) == 0 ) ...` which does not return an indication of equality when command is “exiting”. BTW: The answer's right-parens appear off. – chux - Reinstate Monica Dec 02 '13 at 20:15
0

If you enter a the length of command would be 1. Are you trying to compare the length - 1, so it would be 0. strncmp won't test any character.

I think command has char[MAX_BUFFER] type. You better pass this MAX_BUFFER as third parameter.

Deck
  • 1,969
  • 4
  • 20
  • 41
0

strlen(command) returns the number of non-zero characters in command. It does not include the terminating zero character. Thus, strlen(command) - 1 is one less than the number of non-zero characters. When command contains “a”, strlen(command) - 1 is 0.

Then strncmp("exit", command, strlen(command) - 1) compares zero characters of “exit” to zero characters of command. The result of comparing zero characters is always equality, because an empty string is equal to an empty string. To fix this, do not compare zero characters. Either change the length passed to strncmp to be what you want or use strcmp to compare the entire strings.

If you want to compare the entire string “exit” to the entire string in command, use:

if (strcmp("exit", command) == 0) …

If you want to allow command to be an abbreviation for “exit”, so that the commands “e”, “ex”, “exi”, and “exit” are accepted for “exit”, then use:

if (strncmp("exit", command, strlen(command)) == 0) …
Eric Postpischil
  • 195,579
  • 13
  • 168
  • 312