Reading a file of digits into a linked list

Question

I don't get any error; the code works but it's showing only first element and doesn't show others. I want to show all of the numbers in file with linked list. What is my mistake?

The numbers in numbers.txt:

9 2 3
4 2 3
1 2 9
2 4 8
7 5 9
2 4 7

Here's my code:

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

struct NUMBERS{
    int katsayi;
    int taban;
    int us;
    NUMBERS *next;
};

int main(){
    NUMBERS *temp=NULL;
    NUMBERS *list=NULL; // to keep first element
    NUMBERS *head=NULL; // temp for the first element
    FILE *fp=fopen("numbers.txt","r+");
    while(!feof(fp)){
        temp=(NUMBERS*)malloc(sizeof(NUMBERS));
        fscanf(fp,"%d %d %d",&temp->katsayi,&temp->taban,&temp->us);
        temp->next=NULL;
        if(list==NULL){
            list=temp;
        }
        else{
            head=list;
            while(head->next!=NULL){
                head=head->next;
            }
            head=temp;
        }

    }
    head=list; 
    while(head!=NULL){  
        printf("%d %d %d",head->katsayi,head->taban,head->us);
        head=head->next;
    }
    return 0;
}

Answer 1

There are a few issues here:

• while(!feof(fp)){ is wrong because it tells us when we've read past the end of the file, not when we're at the last line (see link for details). You can check whether the return of fscanf != EOF. I'm assuming the blank lines in your post are a formatting error, but if they aren't, you can use the return value of fscanf to make sure all 3 digits were matched and skip any lines where they weren't.
• The code shouldn't compile because NUMBERS hasn't been typedef'd. No need for this to be in all caps, which is usually reserved for constants.
• No need to cast the result of malloc in C.

• The code to insert a new node:

      head=list;
      while(head->next!=NULL){
          head=head->next;
      }
      head=temp;
  

  doesn't make much sense. After traversing the list, head is simply set to temp, undoing the traversal. We shouldn't be traversing the entire list just to add a single node in any case. The solution is to use a tail node which always points to the last element in the list.

• The code leaks memory. free every malloc.
• Check for errors when opening the file and exit gracefully.
• Consider a LinkedList struct which encapsulates head/tail pointers and has corresponding insertion/removal/traversal/free functions. main should be broken into functions as well (exercise for the reader).

Here's an initial rewrite:

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

typedef struct Numbers {
    int katsayi;
    int taban;
    int us;
    struct Numbers *next;
} Numbers;

int main() {
    Numbers *temp = NULL;
    Numbers *list = NULL; 
    Numbers *tail = NULL; 
    Numbers dummy;
    char file[] = "numbers.txt";
    FILE *fp = fopen(file, "r");

    if (!fp) {
        fprintf(stderr, "%s %d: could not open %s \n", 
                __FILE__, __LINE__, file);
        exit(1);
    }

    while (fscanf(fp, "%d %d %d", 
           &dummy.katsayi, &dummy.taban, &dummy.us) != EOF) {
        temp = malloc(sizeof(*temp));
        memcpy(temp, &dummy, sizeof(*temp));
        temp->next = NULL;

        if (list) {
            tail->next = temp;
        }
        else {
            list = temp;
        }

        tail = temp;
    }

    for (temp = list; temp; temp = temp->next) {
        printf("%d %d %d\n", 
               temp->katsayi, temp->taban, temp->us);
    }

    for (temp = list; temp;) {
        Numbers *prev = temp;
        temp = temp->next;
        free(prev);
    }

    return 0;
}

Output:

9 2 3
4 2 3
1 2 9
2 4 8
7 5 9
2 4 7