strcmp returns wrong value

Question

Can someone explain me why strcmp returns the same value even if passwords are correct/incorrect? I define valid password just below include section and checking it with entered one at the end of my program.

Here's my code:

#include <stdio.h>
#include <signal.h>
#include <string.h>
#include <unistd.h>
#include <stdlib.h>
#include <termios.h>

#define TIME 10
#define MAXPASSWORD 12
#define PASSWORD "pass123"

void sigalrm_handler() {
    printf("\nERR: Time is up...\n");
}

int getch() {
    struct termios oldtc, newtc;
    int ch;
    tcgetattr(STDIN_FILENO, &oldtc);
    newtc = oldtc;
    newtc.c_lflag &= ~(ICANON | ECHO);
    tcsetattr(STDIN_FILENO, TCSANOW, &newtc);
    ch=getchar();
    tcsetattr(STDIN_FILENO, TCSANOW, &oldtc);
    return ch;
}

int main(int argc, char * argv[]) {
    char password[MAXPASSWORD] = {0};
    printf("Enter correct password. You have %d seconds: ", TIME);
    signal(SIGALRM, sigalrm_handler);
    alarm(TIME);
    fflush(stdout);

    for(int i=0; i<MAXPASSWORD; i++)
    {
      password[i] = getch();
      if (password[i] == '\n')
        break;
      printf("*");
    }

    if (strcmp(password, PASSWORD) == 0) {
        printf("\nValid password\n");
    } else {
        printf("\nInvalid password\n");
    }
}

Answer 1

You forgot to replace the \n by NUL

...
for(int i=0; i<MAXPASSWORD; i++)
{
  password[i] = getch();
  if (password[i] == '\n')
  {
    password[i] = 0;   // <<<< add this line
    break;
  }
  printf("*");
}
...

There is another problem: consider what happens if the user enters more than 11 characters before hitting Enter. I let you find out yourself as an exercise.

Answer 2

Problem is you have \n stored into your input buffer so that pass123 and pass123\n will not match.

Thus null terminate the input if you find \n as below.

  if (password[i] == '\n')
  {
     password[i] = '\0';
     break;
  }

Answer 3

You have stated that strcmp() indicates two strings are equal, when you know they are not...

The problems with the \n mentioned in the other answers not withstanding, if you are indeed seeing strcmp() return a wrong indication, the question becomes why?

In C, a string is defined as a null terminated character array. So if for example if you have the following:

char password[MAXPASSWORD] = {0};//where MAXPASSWORD == 12

|p|a|s|s|1|2|3|\0|\0|\0|\0|\0| // `PASSWORD ` - legal string
|s|o|m|e|p|a|s|s|1|2|3|4|\n|  // 'password' 

even after replacing the \n character, this array is too long by one character:

|s|o|m|e|p|a|s|s|1|2|3|4|\0|  // 'password' - too long by 1 character
                        ^     // end of legal definition of `password`

If the password array has too many characters, even in this case after replacing the last char \n with NULL in a location beyond the legal definition of the string, the code becomes subject to undefined behavior.

String functions are designed to work exclusively with strings. When a non-nul terminated character array is presented, the function, in this case strcmp(), because it is looking for the nul terminator to know where the end of a string is, cannot be expected to behave with predictability. (In this case, the location of the nul character would be the cause of undefined behavior.)

To prevent this from happening, even if user were able to enter too many characters in password, always terminate with a statement such as:

password[MAXPASSWORD-1] = 0;  //for a properly initialized array, (as your code indicates)
                              //this guarantees termination occurs 
                              //within legal memory area of defined variable. 

With this, there will be no undefined behavior, and if the strings are different, strcmp() will indicate so.