-1

I'm facing a problem with my code of a simple login program. The problem I'm facing is when I use a switch case or if statement for the option of logging in as an Admin or a User, the input for username is skipped and goes directly to password, and no matter what I type it gives me my error message. Instead, I want it to receive my username first then the password. It works fine on its own if there is only code for either Admin OR User, only one but not when there are more than one. Please help. Note: I'm using the same functions for both admin and user just to check if it works. The picture shows the output.I'm a C newbie, so minimal jargon perhaps? Code as follows:

#include<stdio.h>
#include<conio.h>
#include<stdlib.h>
char username[18];
char pass[16];

void arequest()
{
    printf("\nPlease Enter username:");
    fflush(stdin);
    gets(username);

    printf("\nPlease Enter Password:");
    fflush(stdin);
    gets(pass);
}

void averify()
{
    if (strcmp(username, "admin") == 0)
{
    if (strcmp(pass, "apass") == 0)
        {
            printf("Successful Login");
            _getch();
        }
    else
        {
            printf("Invalid Password");
            _getch;
        }
    }
    else
    {
        printf("Invalid Username");
        _getch();
    }
}

int choice;
int main()
{
    printf("Welcome to Railway Reservation System");
    printf("\n1.Admin \n2.User");
    printf("\nPlease Enter your selection:");
    scanf_s("%d", &choice);
    if (choice == 1) 
    {
        arequest();
        averify();
    }
    else if (choice == 2) 
    {
        arequest();
        averify();
    }
    else 
    {
        printf("Invalid Choice");
        _getch();
        return main;
    }
    return 1;
} 

output

varman37
  • 3
  • 1
  • 8
    `fflush(stdin);` is mainly _Undefined Behavior_. You are allowed to flush output stream only. Take a look at [this SO question](http://stackoverflow.com/questions/18170410/what-is-the-use-of-fflushstdin-in-c-programming) – LPs Dec 15 '16 at 07:43
  • 3
    http://stackoverflow.com/questions/1694036/why-is-the-gets-function-so-dangerous-that-it-should-not-be-used – Ari0nhh Dec 15 '16 at 07:44
  • 3
    As you said that you are a beginner, I have only one remark: this is almost an abstract of what should never be used in C: you use non portable functions from conio and the Microsoft only `fflush` on stdin. You use the almost deprecated and dangerous `gets`. You mix `scanf` and `gets` which always leads to problems because the former leaves the newline in stream while the latter stops on it and gets it. You use global variables when it is not needed... So: only use fgets to read in lines and then sscanf to parse them, only use globals when it is necessaray and all will be fine. – Serge Ballesta Dec 15 '16 at 08:57
  • Your output is not an image, it's plain text. Please [edit] your post and add it as such in your question. – Jongware Dec 15 '16 at 18:26

1 Answers1

0

You are flushing the input stream with fflush(). fflush(stdin) is undefined behavior in most cases, and is at best implementation-dependent. To clear the extra characters from the input stream, consider writing a little function like this:

void clear_stream(void)
{
    int c;

    while ((c = _getch()) != '\n' && c != EOF)
        continue;
}

Remove the calls to fflush(). You do not need to clear the stream after gets(username) since gets() discards the newline. Add a call to clear_stream() after this line in main():

scanf_s("%d", &choice);

There may be extra characters, including a newline, left in the input stream after the call to scanf_s(), and these need to be removed before trying to read user input again. In some cases scanf()_s (and scanf()) will skip over initial whitespaces in reading input, but _getch() and getchar() will not. This illustrates one of the dangers of using scanf().

printf("\nPlease Enter your selection:");
scanf("%d", &choice);
clear_stream();

Also, gets() is considered so dangerous that there is never a reason to use it for anything at all. Use fgets() instead. fgets() does keep the newline, where gets() discards it, so I often write my own version of gets() using fgets() that is safe:

char * s_gets(char *st, int n)
{
    char *ret;
    int ch;

    ret = fgets(st, n, stdin);
    if (ret) {
        while (*st != '\n' && *st != '\0')
            ++st;
        if (*st)
            *st = '\0';
        else {
            while ((ch = getchar()) != '\n' && ch != EOF)
                continue;           // discard extra characters
        }
    }
    return ret;
}

The library conio.h is nonstandard, as are the functions _getch() and scanf_s(). You should use the stdio.h functions getchar() and scanf(). The value returned by scanf() is the number of successful assignments, and you should check this to be sure that the input is as expected. In your program, if the user enters a letter at the selection prompt, no assignment is made, and the value of choice remains uninitialized. The code continues without handling this problem. choice could be initialized to some reasonable value, such as int choice = -1;. Alternatively, you can check the return value from scanf() to see if an assignment was made, and proceed accordingly.

I noticed that you are returning 1 from main(). You should return 0 unless there is an error. And, I see that you return main in the event of an invalid choice. Maybe you meant to return 1 here? And it appears that you have forgotten to #include <string.h> for the strcmp() function.

Finally, I don't understand why username, pass, and choice are global variables. This is a bad practice. These should be declared in main() and passed to functions as needed. It would be a good idea to #define the global constants MAXNAME and MAXPASS instead of hard-coding the array dimensions.

I didn't intend this to be a full-scale code review when I started, but that is what it turned into. Here is a revised version of your program that implements the suggested changes:

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

#define MAXNAME  18
#define MAXPASS  16

void clear_stream(void)
{
    int c;

    while ((c = getchar()) != '\n' && c != EOF)
        continue;
}

char * s_gets(char *st, int n)
{
    char *ret;
    int ch;

    ret = fgets(st, n, stdin);
    if (ret) {
        while (*st != '\n' && *st != '\0')
            ++st;
        if (*st)
            *st = '\0';
        else {
            while ((ch = getchar()) != '\n' && ch != EOF)
                continue;           // discard extra characters
        }
    }
    return ret;
}

void arequest(char username[MAXNAME], char pass[MAXPASS])
{
    printf("\nPlease Enter username:");
    s_gets(username, MAXNAME);

    printf("\nPlease Enter Password:");
    s_gets(pass, MAXPASS);
}

void averify(char username[MAXNAME], char pass[MAXPASS])
{
    if (strcmp(username, "admin") == 0)
{
    if (strcmp(pass, "apass") == 0)
        {
            printf("Successful Login");
            getchar();
        }
    else
        {
            printf("Invalid Password");
            getchar();
        }
    }
    else
    {
        printf("Invalid Username");
        getchar();
    }
}


int main(void)
{
    char username[MAXNAME];
    char pass[MAXPASS];
    int choice;

    printf("Welcome to Railway Reservation System");
    printf("\n1.Admin \n2.User");
    printf("\nPlease Enter your selection: ");
    if (scanf("%d", &choice) == 1) {
        clear_stream();  
        if (choice == 1) 
        {
            arequest(username, pass);
            averify(username, pass);
        }
        else if (choice == 2) 
        {
            arequest(username, pass);
            averify(username, pass);
        }
        else 
        {
            printf("Invalid Choice: %d\n", choice);
            getchar();
            return 1;
        }
    } else {
        clear_stream();             // stream has not yet been cleared
        printf("Nonnumeric input");
        getchar();
    }

    return 0;
}

EDIT

The OP mentioned in the comments that scanf() was causing problems in Visual Studio. Apparently Visual Studio tries to force the use of scanf_s(). The issue with this function is not that it is inherently bad, just that it is nonstandard. One solution might be to use the s_gets() function already added to the code to read the user selection into a character buffer, and then to use sscanf() to extract input. This has an advantage in that there is no need to call the clear_stream() function after s_gets(), because s_gets() cleans up after itself, so the clear_stream() function could now be removed altogether from the program. This can be accomplished with only a small change in main():

char choice_buffer[10];
int choice;

...

if (s_gets(choice_buffer, sizeof(choice_buffer)) &&
    sscanf(choice_buffer, "%d", &choice) == 1) {
    if (choice == 1)

...

} else {
    printf("Nonnumeric input");
    getchar();
}

s_gets() reads up to the first 9 characters (in this case) of a line of user input into choice_buffer, which is an array that will hold chars (there is more space in choice_buffer than is needed to hold a single digit choice and a '\0'). If there is an error, s_gets() returns a NULL pointer, otherwise a pointer to the first char of choice_buffer is returned. If the return value of s_gets() was non-NULL, then sscanf() assigns the first int stored in the buffer to choice. If no int is found in the string, sscanf() returns a value of 0, failing the conditional test.

ad absurdum
  • 19,498
  • 5
  • 37
  • 60
  • Note: replacing `fflush(stdin)` with a function that uses `_getch()`, which is not a standard C library function, lacks portability. – chux - Reinstate Monica Dec 15 '16 at 16:16
  • 1
    @chux-- Yes, I was conflicted about this and the OP's use of nonstandard functions. I think I will revise my answer in a bit in this regard. – ad absurdum Dec 15 '16 at 16:29
  • 1
    Using `clear_stream();` before `gets(username);` only makes sense if _some_ input already resides in `stdin`, else it will consume the input meant for `gets()`. Something MS `fflush(stdin)` does not do. – chux - Reinstate Monica Dec 15 '16 at 16:31
  • Thank you David Bowling for your revised answer. The code works perfectly. I had problems with "scanf" because Visual Studio insisted I use scanf_s instead, and that lead to many more errors. Therefore, I entered #define _CRT_SECURE_NO_WARNINGS at the top of the header files. – varman37 Dec 16 '16 at 05:50
  • If you don't mind, I have questions about void clear_stream(void) and char * s_gets(char *st, int n) – varman37 Dec 16 '16 at 05:53
  • @varman37-- I edited the end of my answer to illustrate the use of `s_gets()` together with `sscanf()`. This allows removal of the `clear_stream()` function altogether, and replaces `scanf()`. `sscanf()` is a standard function, and I believe that it should not cause problems in Visual Studio. – ad absurdum Dec 16 '16 at 07:01
  • @DavidBowling I can't seem to understand the function of " char * s_gets(char *st, int n)" What does it do? Why is it needed for this system? – varman37 Dec 16 '16 at 08:42
  • @varman37-- Read about the standard function `fgets()`. `s_gets()` uses `fgets()` to read at most **n** characters, up to a line of input through the `'\n'`. The result of `fgets()` is a string that may include the `'\n'`, so `s_gets()` removes this `'\n'` if present. Also, if more than **n** characters were in the line of input, there are extra characters in the input stream, so `s_gets()` discards them. It is sort of a combination of `fgets()` and `clear_stream()`, with the safety of specifying the maximum input buffer size. – ad absurdum Dec 16 '16 at 13:51