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 return
ing 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 char
s (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.