Character constant too long for its type (fgets)

Question

void verification() {
    char pass[50];

    printf(" Enter Password : ");
    fgets(pass, 50, stdin);

    if (pass != 'aaanc6400') {   \\ Warning message in here
        printf("\n Invalid Password.. Please enter the correct password. \n\n");
        verification();
    }
    info();
}

When I compile the program, on the marked line it shows warning that "Character constant too long for its type" and also "Comparison between pointer and integer". And then when I run the code and enter the correct password, it still prints that the password is wrong. What am I doing wrong?

@A. Hue. Yes i tried it. Then another warning message comes up "Comparison with string literal results in unspecified behavior" — Anik Shahriar, Nov 14 '16 at 12:21
`'aaanc6400'` is a multi-byte character constant which is certainly not what you want. You need to use double quotes (for c-strings) and use strcmp() to compare them. — P.P, Nov 14 '16 at 12:21
@Rad you only use single quotes on single chars, not on strings (though I guess P.P.'s explanation is better). I don't think though that this is an exact dublicate, even if the answer to the mentioned question solves this case. — Lehue, Nov 14 '16 at 12:24
In addition to previous comments: be aware that the string read by `fgets` ends with a `\n`. So just using quotes instead of apostrophs and `strcmp` is not enough. — Jabberwocky, Nov 14 '16 at 12:34
@AnikShahriar if you find any of the answers helpful, upvote it, if any of them solved your problem - accept by clicking on a gray tick below the answer score. This and many more interesting advices can be found in [ask] and [mcve] — xenteros, Nov 14 '16 at 12:34

xenteros · Answer 1 · 2016-11-14T12:35:51.397

The warning is about you declaring that you have a character of a big length.

'aaanc6400'

is a 9 character long character and the compiler warns you, that it might be a typographical error. It's right.

In C, we use single quote ' for characters and " double quotes for strings which are arrays of characters terminated with '\0' character.

So you have to replace 'aaanc6400' with "aaanc6400" and use strcmp. Remember! fgets might read the \n also, so you can compare the input with "aaanc6400" and "aaanc6400\n" as well. This solution would be sufficient for student project.

score 3 · Answer 2 · answered Nov 14 '16 at 12:19

3

You can't compare a character pointer with a string literal.

What you should rather do is:

if (strcmp(pass, "aaanc6400") == 0)
{ ... }

answered Nov 14 '16 at 12:19

acraig5075

10,588
3
31
50

If you dont remove `\n` from `fgets` then the `strcmp` won't work – artm Nov 14 '16 at 12:30

artm · Accepted Answer · 2016-11-14T22:55:58.170

3

You need to:

Initialise char pass[50] = "";
Remove \n from fgets by pass[strlen(pass) - 1] = '\0'; (after fgets) - that helps you to compare string later on.
if (pass != 'aaanc6400') this one is totally wrong. Use strcmp for string comparison, and double quote for string "aaanc6400"

From @chux: It's better to use strcspn instead of strlen to trim off the \n from fgets

    char pass[50] = "";
    printf(" Enter Password : ");
    if(fgets(pass, 50, stdin) == NULL)
    {
        perror("fgets error");
        return;
    }
    pass[strcspn(pass, "\n")] = 0;  // trim \n

    if(strcmp(pass, "aaanc6400")) {
        printf("\n Invalid Password.. Please enter the correct password. \n\n");
        verification();
    }

edited Nov 14 '16 at 22:55

answered Nov 14 '16 at 12:20

artm

17,291
6
38
54

Tried it. The warning doesn't show up, but everytime i enter the correct password it says invalid password – Anik Shahriar Nov 14 '16 at 12:25
How can i remove '\n'? – Anik Shahriar Nov 14 '16 at 12:26
As I said, add `pass[strlen(pass) - 1] = '\0';` after the `fgets` line – artm Nov 14 '16 at 12:28
1

"need to: Initialize char pass[50] = ""; may be a worthy coding style, but not _needed_. Checking the result of `fgets()` makes for robust programming. `pass[strlen(pass) - 1] = '\0';` is a hacker exploit should the user enter a null character as the first character. Better to use [this](http://stackoverflow.com/q/2693776/2410359) – chux - Reinstate Monica Nov 14 '16 at 17:14
thanks @chux I saw your comment on that answer - indeed I guess not many people know about `strcspn` (I don't even know what it stands for :) – artm Nov 14 '16 at 22:36
hehe seems like it stands for `string character span` ?? - the standard should really add some description like that.. – artm Nov 14 '16 at 22:39
last point `char pass[50] = "";` I came to that just like a normal `int i = 0;` true that sometimes it's not needed, but rule of thumb is to initialize all variables before use, even for array type. – artm Nov 14 '16 at 22:42
1

@artm: `strcspn` stands for *string complement span*, `strspn` for *string span*. These functions are very handy for parsing strings non destructively. – chqrlie Nov 14 '16 at 22:48
`if(0 != strcmp(pass, "aaanc6400"))` looks awful and totally useless. The reverse notation to avoid erroneous assignments in test expressions is obsolete as modern compilers can produce warnings for this and many other potential programming errors. Always use `-Wall` or equivalent. – chqrlie Nov 14 '16 at 22:51
The recursion approach is ill conceived: the return value of `fgets()` is not tested, so end of file will cause endless recursion with dire consequences. – chqrlie Nov 14 '16 at 22:55
@artm Recall that if `fgets()` returns `NULL` due to a rare input error, the content of `pass` is indeterminate. This negates any subsequent benefit of the initialization `pass[50] = ""`. – chux - Reinstate Monica Nov 14 '16 at 23:00
@chqrlie agree and edited - thanks for the `strcscpn` abbreviation comment. I think the part **non destructively** should be bold. – artm Nov 14 '16 at 23:02
1

@chux - true we should always check return value – artm Nov 14 '16 at 23:03
@artm: sorry for nagging, but `return;` is inappropriate in case of end of file as the caller has no way to tell if the `verification()` succeeded. The API should be changed IMHO. – chqrlie Nov 14 '16 at 23:07

score 1 · Answer 4 · answered Nov 14 '16 at 23:04

Multiple problems in your verification function:

'aaanc6400' is a multi-character character constant, an obsolete construction that cannot be used portably. You probably meant to compare the string read from the user with the string "aaanc6400": you should use strcmp() for this.
You should check the return value of fgets(): on end of file or read error, it returns NULL and the array contents are indeterminate.
You should use a loop instead of recursion in case of error.

Here is a corrected version:

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

void verification(void) {
    char pass[50];

    printf(" Enter Password: ");
    for (;;) {
        fflush(stdout);
        if (fgets(pass, 50, stdin) == NULL) {
            printf("unexpected end of file\n");
            exit(1);
        }
        pass[strcspn(span, "\n")] = '\0'; // remove the newline if present
        if (strcmp(pass, "aaanc6400") == 0) {
            // correct password, stop prompting.
            break;
        }
        printf("\n Invalid Password. Please enter the correct password: ");
    }
    info();
}

Character constant too long for its type (fgets)

4 Answers4