Why is my call to strcmp not working as expected?

Question

I'm new to C and programming in general. It's a guessing game where the user has a certain amount of guesses to guess whatever the hidden message is.

Unfortunately, even if I enter the correct word, it does not register it as being correct.

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


Lost(){
    printf("You lost");
}

Won(){
    printf("You won");
}

int main(void)
{
    char HiddenMessage[7] = "Cameron";
    char Guess[50] = "";
    int Tries = 0;
    int MaxTries = 5;

    while(Tries != MaxTries && strcmp(HiddenMessage, Guess) != 0){
        printf("Guess the secret word:");
        scanf("%s", Guess);
        Tries++;
        if (Tries == MaxTries){
            Lost();
        }
        else if (strcmp(HiddenMessage, Guess) == 0){
            Won();
        }
    }
    return 0;
}

How do I structure this to ensure that if a Guess occurs correctly within the number of tries it runs Won();

Answer 1

You should have received a warning about:

char HiddenMessage[7] = "Cameron";

if your compiler was doing its job properly :-)

Seven characters is not enough to store Cameron and the end-of-string marker. Try it again with:

char HiddenMessage[] = "Cameron";

This will make sure enough characters are set aside for the full string.

---

If you're interested, this is covered in C11 6.7.9 Initialization /14 (emphasis added):

An array of character type may be initialized by a character string literal or UTF−8 string literal, optionally enclosed in braces. Successive bytes of the string literal (including the terminating null character if there is room or if the array is of unknown size) initialize the elements of the array.

So what your code will give you is the sequence of characters { 'C', 'a', 'm', 'e', 'r', 'o', 'n' } but with no \0 at the end to make it a C string. That means strcmp cannot be guaranteed to give you the results you want (it will possibly run off the end of the array during the comparison).

---

And, as an aside, scanf with the unbounded %s format specifier is considered rather dangerous in non-trivial code, since there's no way to protect against buffer overflow.

If you're looking for a fairly bullet-proof user input function that can detect and mitigate this, have a look at this earlier answer of mine.

It allows prompting, prevents buffer overflow, properly handles line-based user input (no half lines delivered to user) and so on.

It's what I use for simple console applications (where no better user input method is available).

Answer 2

There are, essentially, four errors in your code:

1. As paxdiablo has stated, your HiddenMessage array is not large enough to hold the text you have given plus the nul terminator.
2. You need to define (or at least declare) your Lost and Won functions before you use them.
3. The definitions of these two functions are incorrect, as you have not specified a return type: void is suitable for functions that don't return anything.
4. You must include stdio.h to get the definitions of printf and scanf

Here's a fixed (and working) version of your code with the corrections made:

#include <stdlib.h>
#include <stdio.h>  // MUST include this for "printf" and "scanf" functions

// You can keep these DEFINITIONS after "main" if you like, but then you'll need to 
// have "pre-declarations" of them (the two commented lines immediately below).
// void Lost();
// void Won();

void Lost() {
    printf("You lost");
}

void Won() {
    printf("You won");
}

int main()
{
    char HiddenMessage[8] = "Cameron"; // Added enough space for the nul terminator
    char Guess[50] = "";
    int Tries = 0;
    int MaxTries = 5;

    while (Tries != MaxTries && strcmp(HiddenMessage, Guess) != 0) {
        printf("Guess the secret word:");
        scanf("%s", Guess);
        Tries++;
        if (Tries == MaxTries) {
            Lost();
        }
        else if (strcmp(HiddenMessage, Guess) == 0) {
            Won();
        }
    }

    return 0;
}

Feel free to ask for further clarification and/or explanation.

Answer 3

Another significant problem is your failure to validate any of the input. Any time (means ANY time) you take input, you must validate the input succeeds before relying on the data. Failure to validate invites undefined behavior.

Validations are simple, if you read input with a function check the return to validate you got all the input you were expecting. For example:

        printf ("  try no. %d - Guess the seceret word: ", tries + 1);
        if (scanf ("%49s", guess) != 1) {   /* read/validate word */
            fputs ("(error: EOF encountered reading guess.)\n", stderr);
            return 1;
        }

(note: the use of the field-width modifier to limit the number of characters that can be read. Failing to control the number of characters than can be input will read to attempt to write beyond the bounds of your array if the user enters 50 (or more) characters.)

Your logic is a bit awkward. All you need to do is loop continually until the user either wins the game or exhausts the number of tries available. Rearranging your logic slightly, you could do something like:

(Edited to empty-stdin)

    printf ("HANGMAN - you have %d tries to guess the word.\n\n", MAXTRIES);

    for (;;) {  /* loop continually until win or tries exhausted */
        printf ("  try no. %d - Guess the seceret word: ", tries + 1);
        if (scanf ("%49s", guess) != 1) {   /* read/validate word */
            fputs ("(error: EOF encountered reading guess.)\n", stderr);
            return 1;
        }
        if (strcmp (hiddenmsg, guess) == 0) /* compare hiddenmsg & guess */
            break;                          /* if matched, break loop */
        if (++tries == MAXTRIES) {          /* test if all tries exhausted */
            fputs ("\nyou have exhausted all guesses - you lost :(\n", stderr);
            return 1;
        }
        /* empty stdin */
        for (int c = getchar(); c != EOF && c != '\n'; c = getchar()) {}
    }
    puts ("\ncongratulations - you won.\n");  /* issue congrats & exit */

Putting it altogether in an example, you could do:

#include <stdio.h>
#include <string.h>

#define MAXTRIES  5
#define MAXGUESS 50

int main (void) {

    char hiddenmsg[] = "Cameron",
        guess[MAXGUESS] = "";
    int tries = 0;

    printf ("HANGMAN - you have %d tries to guess the word.\n\n", MAXTRIES);

    for (;;) {  /* loop continually until win or tries exhausted */
        printf ("  try no. %d - Guess the seceret word: ", tries + 1);
        if (scanf ("%49s", guess) != 1) {   /* read/validate word */
            fputs ("(error: EOF encountered reading guess.)\n", stderr);
            return 1;
        }
        if (strcmp (hiddenmsg, guess) == 0) /* compare hiddenmsg & guess */
            break;                          /* if matched, break loop */
        if (++tries == MAXTRIES) {          /* test if all tries exhausted */
            fputs ("\nyou have exhausted all guesses - you lost :(\n", stderr);
            return 1;
        }
        /* empty stdin */
        for (int c = getchar(); c != EOF && c != '\n'; c = getchar()) {}
    }
    puts ("\ncongratulations - you won.\n");  /* issue congrats & exit */
}

Example Use/Output

Typical try and fail game:

$ ./bin/hangman
HANGMAN - you have 5 tries to guess the word.

  try no. 1 - Guess the seceret word: Alligator
  try no. 2 - Guess the seceret word: Campers
  try no. 3 - Guess the seceret word: Scam
  try no. 4 - Guess the seceret word: Horses
  try no. 5 - Guess the seceret word: Bananna

you have exhausted all guesses - you lost :(

A lucky win:

$ ./bin/hangman
HANGMAN - you have 5 tries to guess the word.

  try no. 1 - Guess the seceret word: Lightning
  try no. 2 - Guess the seceret word: Thunder
  try no. 3 - Guess the seceret word: Geroge
  try no. 4 - Guess the seceret word: Cameron

congratulations - you won.

(Testcase After Edit w/Example from Comment)

$./bin/hangman
HANGMAN - you have 5 tries to guess the word.

  try no. 1 - Guess the seceret word: xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxCameron
  try no. 2 - Guess the seceret word: foo
  try no. 3 - Guess the seceret word: Cameron

congratulations - you won.

Look things over and let me know if you have any further questions.

Answer 4

As strcmp compares the difference between each character till the end of string that is \0 So your buffer needs to be large enough to hold your string i.e. char HiddenMessage[8] = "Cameron";

PS you can avoid a lot of lengthy code

#include <stdlib.h>

int main()
{
    char HiddenMessage[8] = "Cameron";
    char Guess[50] = "";
    int Tries = 0;
    int MaxTries = 5;

    while (Tries != MaxTries)
    {
        printf("Guess the secret word:");
        scanf("%s", Guess);
        Tries++;
        if (strcmp(HiddenMessage, Guess) == 0)
        {
            Won();
            return 0;
        }
    }
    Lost();
    return 0;
}

void Lost()
{
    printf("You lost");
}

void Won()
{
    printf("You won");
}

Answer 5

There are multiple problems with your code. The size of HiddenMessage reported by @paxdiablo is just one of them.

• You should use fgets instead of scanf because scanf won't consume the newline. You will be stuck on the second iteration.

• You increment Tries and test it against MaxTries before testing if the guess was correct. As a consequence the program will tell that the user lost before testing the validity of the last guess.

• Once the user guessed the word and won, you must break from the while loop and terminate the program. With your code, after the program reported that the user won, it will ask for another guess if it wasn't the last guess.