Why is fgets() and strncmp() not working in this C code for string comparison?

Question

This is a very fun problem I am running into. I did a lot of searching on stack overflow and found others had some similar problems. So I wrote my code accordingly. I originally had fscan() and strcmp(), but that completely bombed on me. So other posts suggested fgets() and strncmp() and using the length to compare them.

I tried to debug what I was doing by printing out the size of my two strings. I thought, maybe they have /n floating in there or something and messing it up (another post talked about that, but I don't think that is happening here). So if the size is the same, the limit for strncmp() should be the same. Right? Just to make sure they are supposedly being compared right. Now, I know that if the strings are the same, it returns 0 otherwise a negative with strncmp(). But it's not working.

Here is the output I am getting:

perk
repk
Enter your guess: perk
Word size: 8 and Guess size: 8
Your guess is wrong
Enter your guess: 

Here is my code:

void guess(char *word, char *jumbleWord)
{
        size_t wordLen = strlen(word);
        size_t guessLen; 
        printf("word is: %s\n",word);
        printf("jumble is: %s\n", jumbleWord);


        char *guess = malloc(sizeof(char) * (MAX_WORD_LENGTH + 1));
        do
        {
            printf("Enter your guess: ");
            fgets(guess, MAX_WORD_LENGTH, stdin);
            printf("\nword: -%s- and guess: -%s-", word, guess); 
            guessLen = strlen(guess);
            //int size1 = strlen(word);
            //int size2 = strlen(guess); 

            //printf("Word size: %d and Guess size: %d\n",size1,size2);


            if(strncmp(guess,word,wordLen) == 0)
            {
                printf("Your guess is correct\n"); 
                break; 
            }

            }while(1);
    }

I updated it from suggestions below. Especially after learning the difference between char * as a pointer and referring to something as a string. However, it's still giving me the same error.

Please note that MAX_WORD_LENGTH is a define statement used at the top of my program as

#define MAX_WORD_LENGTH 25

Answer 1

Use strlen, not sizeof. Also, you shouldn't use strncmp here, if your guess is a prefix of the word it will mistakenly report a match. Use strcmp.

Answer 2

sizeof(guess) is returning the size of a char * not the length of the string guess. Your problem is that you're using sizeof to manage string lengths. C has a function for string length: strlen.

sizeof is used to determine the size of data types and arrays. sizeof only works for strings in one very specific case - I won't go into that here - but even then, always use strlen to work with string lengths.

You'll want to decide how many characters you'll allow for your words. This is a property of your game, i.e. words in the game are never more that 11 characters long.

So:

// define this somewhere, a header, or near top of your file
#define MAX_WORD_LENGTH 11

// ...

size_t wordlen = strlen(word);
size_t guessLen;

// MAX_WORD_LENGTH + 1, 1 more for the null-terminator:
char *guess = malloc(sizeof(char) * (MAX_WORD_LENGTH + 1));

printf("Enter your guess: ");
fgets(guess, MAX_WORD_LENGTH, stdin);

guessLen = strlen(guess);

Also review the docs for fgets and note that the newline character is retained in the input, so you'll need to account for that if you want to compare the two words. One quick fix for this is to only compare up to the length of word, and not the length of guess, so: if( strncmp(guess, word, wordLen) == 0). The problem with this quick fix is that it will pass invalid inputs, i.e. if word is eject, and guess is ejection, the comparison will pass.

Finally, there's no reason to allocate memory for a new guess in each iteration of the loop, just use the string that you've already allocated. You could change your function setup to:

char guess(char *word, char *jumbledWord)
{
    int exit;

    size_t wordLen = strlen(word);
    size_t guessLen; 

    char *guess = malloc(sizeof(char) * (MAX_WORD_LENGTH + 1));

    do
    {
        printf("Enter your guess: ");
        // ...

Answer 3

As everyone else has stated, use strlen not sizeof. The reason this is happening though, is a fundamental concept of C that is different from Java.

Java does not give you access to pointers. Not only does C have pointers, but they are fundamental to the design of the language. If you don't understand and use pointers properly in C then things won't make sense, and you will have quite a bit of trouble.

So, in this case, sizeof is returning the size of the char * pointer, which is (usually) 4 or 8 bytes. What you want is the length of the data structure "at the other end" of the pointer. This is what strlen encapsulates for you.

If you didn't have strlen, you would need to dereference the pointer, then walk the string until you find the null byte marking the end.

i = 1;
while(*guess++) { i++ }

Afterwards, i will hold the length of your string.

Update:

Your code is fine, except for one minor detail. The docs for fgets note that it will keep the trailing newline char.

To fix this, add the following code in between the fgets and strncmp sections:

if ( guess[guessLen-1] == '\n' ) {
    guess[guessLen-1] = '\0'; 
}

That way the trailing newline, if any, gets removed and you are no longer off by one.

Answer 4

Some list of problems / advices for your code, much too long to fit in a comment:

• your function returns a char which is strange. I don't see the logic and what is more important, you actually never return a value. Don't do that, it will bring you trouble
• look into other control structures in C, in particular don't do your exit thing. First, exit in C is a function, which does what it says, it exits the program. Then there is a break statement to leave a loop.

A common idiom is

do {

   if (something) break;
} while(1)

• you allocate a buffer in each iteration, but you never free it. this will give you big memory leaks, buffers that will be wasted and inaccessible to your code
• your strncmp approach is only correct if the strings have the same length, so you'd have to test that first