Please Confirm

Hi Guys, I am a beginner programmer in C. I just wrote this simple program and its compile and execute just fine. I just want to know if its a good practice to do what I did on line 17 where I have the return 0;. Please assist me.

Code:

---

#include <stdio.h>

int main (void)
{
    char machine_password[] = "HELLO";
    char user_password[10];
    int attempts = 1;
   
    while (attempts <= 3){
          printf("Please Enter Your Password\n");
          scanf("%s", &user_password);
         
          if (!strcmp(user_password, machine_password)){
                            printf("WELCOME TO YOUR MAIL\n");
                           
                            system ("PAUSE");
                            return 0;
                            }
         
          else if(attempts <= 3){
                      printf("INCORRECT PASSWORD\n");
                      }
                     
          attempts = attempts +1;         
          }
    system("PAUSE");
    return 0;   
}

---

Do you mean putting a return statement in the middle of a function? It's mostly a style issue -- it's perfectly valid C code. Some people say it's bad style, as are using the 'break' and 'continue' keywords. They feel it's similar to using goto (note, there are good reasons to use goto also).

Personally I think it's okay as long as it's not used excessively, easy to read, etc. One alternative is to have the code that runs when you don't have an error end up nested inside a bunch of if statements. That makes your code indented a whole bunch, which I find ugly and not very easy to read. Again, that's a style preference.

The only real problem I see with your code is the system("PAUSE"); commands, which are not portable (i.e. Windows-specific). Learn a better way to FAQ > Stop my Windows Console from disappearing everytime I run my program? - Cprogramming.com.

it legal to do that. whether its good is a style issue, but I think its ok if at that point your program is done and it is difficult to exit the loop and skip any subsequent code to get to the return at the end of the function. sometimes the logic required to get to the end is uglier than issuing a return on the spot. that said, in your case you could have just done

Code:

---

          if (!strcmp(user_password, machine_password)){
                            printf("WELCOME TO YOUR MAIL\n");
                            break;
                            }

---

which would have the same effect, namely exit the loop, do a system(pause) and return 0;

To follow up with dmh2000's post, the fact that the logic is the same whether they enter the correct username/password may also be a problem. You may want to return 1; if they fail the login attempt, so whatever uses this program to check credentials can react accordingly.

Another thing that is worth mentioning is that the way you are currently reading in the password is not desirable in a real situation. Why? Well because you are not specifying a limit to how much you are going to read as input, even if that exceeds the available space in your password storage array, therefore almost inviting a buffer overflow attack.

Think about the potential consequences of a user typing in a string of 100 characters when prompted for the password. What would happen? Why?

Just some food for thought.