C - Hashtable Doesn't Work Properly

Question

Here is my code:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#define m 9



int flag1=1;
//Creating memory for username and password
struct node // The list that is being used to store username and password
{
    char username[30];
    char password[30];
    struct node *next;
};

int getkey(char[30]); //a method to get "randomly" a key
int hash(int);  // a method to create the hashcode
void insert(struct node **,char[30],char[30]); //adding a node struct to a specific possition in hashtable
void initializeHashTable(struct node **);
void search(struct node **,char[30]);//check if password exists
void searchuser(struct node **,char[30]); //check if username exists
void print(struct node**);//printing the hashtable

void print(struct node **T)
{
     struct node *tmp;
     int i,flag=0;
     printf("-------Data Base-------\n");
     for(i=0;i<m;i++)
     {
         tmp=*(T+i);
         while(tmp!=NULL)
         {
             printf("Username: %s  Password: %s\n",tmp->username,tmp->password);
             tmp=tmp->next;
         }

     }
}
void search(struct node **T,char password[30])
{
    struct node *tmp;
    int i,flag=0;
    for(i=0; i<m; i++)
    {
        tmp=*(T+i);
        while(tmp!=NULL)
        {
            if(strcmp(tmp->password,password)==0)
            {
                flag=1;
                break;
            }
            tmp=tmp->next;

        }
        if(flag==1)
            break;

    }
    if(flag==1)
         printf("Authentication Successfull\n");
    else
       printf("Authentication Failed\n");

}

void searchuser(struct node **T,char user[30])
{
    struct node *tmp;
    int i,flag=0;
    for(i=0; i<m; i++)
    {
        tmp=*(T+i);
        while(tmp!=NULL)
        {
            if(strcmp(tmp->username,user)==0)
            {
                flag=1;
                break;
            }
            tmp=tmp->next;

        }
        if(flag==1)
            break;

    }
    if(flag==0)
      {
           printf("Username NOT Found\n");
           flag1=0;

      }
}

void initializeHashTable(struct node **T)
{
    int i;
    for (i=0; i<m; i++)
        *(T+i)=NULL;

}
int getkey(char name[30])
{


    int i=0;
    int key=0;
    for (i=0; i<15; i++)
    {
        key+=name[i];

    }
    return key;

}

int hash(int key)
{
    return key%m;
}

void insert (struct node **T,char name[30],char password[30])
{

    struct node *newnode;
    newnode=(struct node*)malloc(sizeof(struct node));
    strcpy(newnode->username,name);
    strcpy(newnode->password,password);
    int key;
    key=getkey(name);
    int hashcode;
    hashcode=hash(key);
    //printf("Key:%d\n",key);
   // printf("Hashcode:%d\n",hashcode);

    if(*T+hashcode==NULL)
    {
        (*(T+hashcode))->next=newnode;
        newnode->next=NULL;
    }
    else
    {

        newnode->next=(*(T+hashcode));
        (*(T+hashcode))=newnode;
    }



}



int main()
{//possible content of file: phill 1234
    char choice;
    struct node **T;
    struct node **head;
    head==NULL;
    T=(struct node**)malloc(m*sizeof(struct node));//creating the hashmap
    FILE *fp;
    char name[30];
    char pass[30];

    fp=fopen("passwords.txt","r");

    if (fp==NULL)
    {
        printf("File Not Found");
        return 0;
    }
    else
    {
        initializeHashTable(&(*T));
        while(!feof(fp)) // end of file
        {
            fscanf(fp,"%s %s",name,pass); //reads until first wild space,
            //one each loop, you get 1 username and 1 password from the file
            //adding them on hashmap
            insert(&(*T),name,pass);


        }
    }

    //user authentication
    do{

        printf("Enter Username:\n");
        fflush(stdin);
        scanf("%s",&name);
        searchuser(&(*T),name);
        if(flag1==1)
        {
        printf("Enter Password:\n");
        scanf("%s",&pass);
        search(&(*T),pass);

        }
        printf("Try Again? (y/n)\n");
        fflush(stdin);
        scanf("%d",&choice);
    }while(choice!='y');

    free(T);
    free(fp);
    free(head);



    }

---

• My code works fine on the loop , it does the authentication successfully but then sometimes it crashes, or i give the same details(username and password) and somehow the authentication fails

• part of the code that i believe its buggy:

      //user authentication
      do{
      printf("Enter Username:\n");
      fflush(stdin);
      scanf("%s",&name);
      searchuser(&(*T),name);
      if(flag1==1)
      {
      printf("Enter Password:\n");
      scanf("%s",&pass);
      search(&(*T),pass);
  
      }
      printf("Try Again? (y/n)\n");
      fflush(stdin);
      scanf("%d",&choice);
  }while(choice!='y');
  

  if i put do..while(choice=='y') which means run the loop while choice=='y'it stops if choice=='y' which is kinda odd

Issue: Authenticator doesn't work properly after 1st try

Thanks A Lot For Your Time!

Answer 1

Several issues that jump out at me:

struct node **T;
...
T=(struct node**)malloc(m*sizeof(struct node));

Do you intend for T to be a sequence of struct node (in which case the type of T is wrong), or a sequence of pointers to struct node (in which case, the argument to sizeof is wrong)?

It would be a lot cleaner (and much less error-prone) if you wrote

T = malloc( m * sizeof *T ); 

then it's just a matter of deciding the type of T. Personally, I would expect you to be allocating a sequence of struct node, so that would likely be declared

struct node *T = malloc(  m * sizeof *T );

However, the rest of your code really seems to assume that T is a sequence of pointers to struct node, so in that case that would be

struct node **T = malloc( m * sizeof *T );

That's the beauty of this idiom - the only thing you need to change is the type of T. The malloc call itself doesn't need to change. If the type of T is struct node *, then sizeof *T is equivalent to sizeof (struct node). If T is struct node **, then sizeof *T is equivalent to sizeof (struct node *).

Then there's this:

initializeHashTable(&(*T));

which should really just be

intializeHashTable( T );

Also,

while(!feof(fp))

is always wrong - feof won't return true until after you attempt to read past the end of the file, so you'll wind up looping once too often. You should check against the result of the input operation instead:

while ( fscanf(fp,"%s %s",name,pass) == 2 )
{
   insert(T,name,pass);
}

Again, the argument &(*T) should just be T.

As for your input:

printf("Enter Username:\n");
fflush(stdin);
scanf("%s",&name);

The fflush call is unnecessary here - if there's an input operation immediately following an output operation, a flush is implied.

Edit

I'm embarrased to say that I misread that fflush call - for some reason I read that as fflush( stdout), meaning you'd want to make sure your output was written to the console before calling scanf.

Calling fflush on an input stream is erroneous, and the behavior on doing so is undefined. It won't clear the input stream of any unread input (except in MSVC, and that's because Microsoft decided to codify existing bad behavior).

Lose the fflush call altogether.

End edit

The scanf call should be

scanf( "%s", name );

The & is unnecessary here - name will implicitly be converted from type "30-element array of char" to "pointer to char". Same for pass.

Edit

As written, these scanf calls are unsafe - if you type in a string that's longer than the target buffer, scanf will happily write those extra characters to the memory immediate following that buffer, resulting in anything from corrupted data to a seg fault. You need to make sure you don't read too many characters. You can do that either with a field width specifier, such as

scanf( "%29s", name );  // leave at least one element for the 0 terminator

or use fgets:

fgets( name, sizeof name, stdin );

The fgets call will read and store the trailing newline to the buffer if there's room, so you'll need to do some extra work:

char *newline = strchr( name, '\n' );
if ( *newline )
  *newline = 0;

If there's no newline in the buffer, then you will want to clear the input stream before the next read:

while ( getchar() != '\n' )
  ; // empty loop

End edit

  scanf("%d",&choice);
}while(choice!='y');

You're using the wrong format specifier to read choice - you're telling scanf to expect a decimal integer input, not a character. 'y' doesn't match up with the format, so scanf doesn't actually read it from the input stream, and choice is not updated. You should change that to

  choice = getchar();
} while ( choice != 'y' );

There are plenty of other issues, but start with those.