Infinite loop when reading from file in C

Question

Sorry this is a bit lengthy, but I've been stuck for hours now on this pesky bug from my code, and I'm really desperate.

I'm writing a basic Rolodex program. The Rolodex is a linked list, each node of which stores a person's first name, last name, and phone number (all as strings). The initial contents of the Rolodex are read from a file that the user specifies, for example:

bash $ ./rolodex bobsRolodex

This rolodex would use the file "bobsRolodex." If the user doesn't specify a file, it defaults to "myRolodex." The content of each file, if there is any, is added to the linked list at the start of the program. Then the user is asked to either input a new contact, print the list, or quit. When the user quits, the linked list updates the file by overwriting the previous content of the file, always following the same format, which is, on consecutive lines:

first last phone

The problem I'm having is that if there is content in the file I'm specifying, reading from the file and trying to insert the content into the linked list results in an infinite loop. Here is the code; I put a comment at the function that's giving me the problem:

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

struct Rolodex
{
    char first[40];
    char last[40];
    char phone[40];
    struct Rolodex *next;
};

typedef struct Rolodex r_struct;
r_struct *head = NULL;

void add_card_to_rolodex(r_struct* card)
{
    if(head == NULL)
    {
        card -> next = NULL;
        head = card;
    }
    else
    {
        card -> next = head;
        head = card;
    }
}

void open_and_read_from_file(char* fileName) //PROBLEM IS HERE!!!
{
    FILE* ifp = fopen(fileName, "r");
    if(ifp) //if file exists
    {
        r_struct* card = malloc(sizeof(r_struct));
        card->next = NULL;
        while(fscanf(ifp, "%s %s %s\n", card->first, card->last, card->phone) == 3)
        {
            add_card_to_rolodex(card);
        }
        fclose(ifp);
    }
}

void write_to_file(char* fileName)
{
    FILE* ofp = fopen(fileName, "w");
    r_struct* temp = head;
    while(temp != NULL)
    {
        fprintf(ofp, "%s %s %s\n", temp->first, temp->last, temp->phone);
        temp = temp->next;
    }
    fclose(ofp);
}

void print_rolodex(bool terminal)
{
    int count = 0;
    r_struct *temp = head;
    while(temp != NULL)
    {
        if(terminal)
        {
            printf("%d ", count);
            count++;
        }
        printf("%s %s %s\n", temp->first, temp->last, temp->phone);
        temp = temp->next;
    }
}

char read_command(char* fileName)
{
    char command;
    printf("%s Command: ", fileName);
    scanf("%c", &command);
    getchar();
    return command;
}

void evaluate_command(char command)
{
    if(toupper(command) == 'I') //insert new card
    {
        r_struct *card = malloc(sizeof(r_struct));
        card -> next = NULL;

        printf("Enter card: first-name last-name phone:\n");
        scanf("%s %s %s", card->first, card->last, card->phone);
        getchar();

        add_card_to_rolodex(card);
    }
    else if(toupper(command) == 'P') //print all cards
    {
        bool terminal = true;
        print_rolodex(terminal);
    }
}

void deallocate_memory()
{
    r_struct* temp = head->next;
    if(head != NULL)
    {
        while(temp != NULL)
        {
            free(head);
            head = temp;
            temp = temp->next;
        }
    }
}

int main(int argc, char *argv[])
{
    char *fileName = argv[1];
    char command;

    if(fileName == NULL)
        fileName = "myRolodex";

    open_and_read_from_file(fileName); //PROBLEM IS HERE

    while(command != 'Q')
    {
        command = read_command(fileName);
        evaluate_command(command);
    }

    write_to_file(fileName);
    deallocate_memory();
    return 0;
}

Answer 1

Essentially, you need to rearrange things so

r_struct* card = malloc(sizeof(r_struct));

happens for each card you're adding to the rolodex. Currently you're overwriting the same block of memory as you're only making one call to malloc.

Alternatively take a deep copy of the structure in add_card_to_rolodex.

Don't forget to clean up the memory though with calls to free when you want to destroy your linked list.