Reading from file into a struct in C

Question

I am doing a small project for college (1st semester doing a bookstore implementation) and I have a problem with reading from a text file into a list of structs, with a two-dimensional array of characters in it that stores authors. However, it doesn't work properly (every time I launch the program it shows list is empty). Writing to a file works (I think, because it overwrites my text file with empty data).

Example data:

Adam Mickiewicz///Pan Tadeusz/Publisher 1/1833/24.99
Jules Verne///Around The World in 80 days/Publisher 1/1904/19.99
Jean-Jacques Sempe/Rene Goscinny//Little Nicholas/Publisher 2/1963/22.99

My structure:

#define AK 3 // Constant denoting max number of authors

typedef struct
{
    char authors[AK][100];
    char title[255];
    char publisher[100];
    unsigned int year;
    double price;

    struct Book *next;
} Book;

Book *first; // Address of the first element in a list

Reading from file:

void read_from_file(const char *path)
{
    int no_of_authors;
    int i; 
    printf("Loading...\n"); 
    FILE *fp = fopen(path, "r"); // Opening a file

    // Checking for errors
    if (!fp) {
        printf("Error reading from file!");
        return;
    }

    // The loading mechanism 
    no_of_authors = 0;
    while (!feof(fp)) {
        Book *new = (Book*) malloc(sizeof(Book));
        for (i = 0; i < AK; i++) {
            fscanf(fp, "%s/", new->authors[i]);
        }
        fscanf(fp, "%s/%s/%u/%lf", new->title, new->publisher,
                &new->year, &new->price);
        fscanf(fp, "\n");
        new = new->next;
    }

    fclose(fp);
    printf("Loading successful.");
}

Writing to file (just in case):

void write_to_file(const char *path, Book *first)
{
    int i;
    printf("Saving...\n");
    FILE *fp = fopen(path, "w");
    Book* current = first;

    if (!fp) {
        printf("Error opening the file!");
        dump_list(first); // Dumping the list to prevent memory leaks, this works okay
    }
    // Saving mechanism
    while (first != NULL) {
        for (i = 0; i < AK; i++) {
            fprintf(fp, "%s/", current->authors[i]);
        }
        fprintf(fp, "%s/%s/%u/%lf", current->title, current->publisher,
                &current->year, &current->price);
        fprintf(fp, "\n");
    }

    fclose(fp);
    printf("Saved successfully");
}

Answer 1

OP's biggest failing is not checking the return value of fscanf(). Had code done so, problems would be more rapidly detected.

---

When is comes to reading lines of data the first consideration is:

Could input be faulty?

With learner applications this is often considered no. Input is either "good" or end-of-file. Let us not assume data is too well formated.

As it turns out, while the data file may not be faulty, the code reading it may be wrong. The subtle 2nd reason for code to check the *scanf() return values - self validation.

---

For line orientated data, it is much better to read is a line of data with fgets() than feof(), fscanf()... See also @Paul Ogilvie

 char buf[sizeof(Book) * 2]; // Use an ample sized buffer
 while (fgets(buf, sizeof buf, fp)) {

---

Use "%s" to read in text that does not include white-space. This will also read in '/'. Since '/' is use to delimit, "%s" is not an acceptable input specifier. Further names like "Adam Mickiewicz" include a space. A 2nd reason to not use "%s".

Consider what fscanf(fp, "%s/", new->authors[i]); is doing. "%s" scans into new->authors[i] non-white-space characters. A character after non-white-space characters is a white-space, never a '/'.

Use "%[^/]" to read in text that does not include '/'.

Use "%n" to keep track of the current scan offset.

Now parse the line.

    char *p = buf;
    Book *nu = malloc(sizeof *nu);  // no cast needed
    for (size_t i = 0; i < AK; i++) {
      int n = 0;
      sscanf(p, "%99[^/]/%n", nu->authors[i], &n);
      if (n == 0) {
        Handle_Parse_Error();
      }
      p += n; 
    }
    if (sscanf(p, "%254[^/]/%99[^/]/%u/%lf", 
        nu->title, nu->publisher, &nu->year, &nu->price) != 4) {
      Handle_Parse_Error();
    }

Robust code would add checks on each member. Suit to coding goals.

    if (nu->year < -6000 || nu->year > 2999) Fail_Year_Range();

---

Further work is needed to link data together. OP's code is unclear on this matter and so left to OP. A possible approach:

Book *read_from_file(const char *path) {
  Book first;  // Only the .next member is used.
  first.next = NULL;
  Book *current = &first;

  // setup while () loop
  ...
  while (fgets(buf, sizeof bu, fp)) {
    ...
    Book *nu = malloc(sizeof *nu);
    nu->next = NULL;
    ...
    // parse data, fill in rest of nu->
    ....
    current->next = nu;  // update current pointer
    current = nu;
  }
  // close up file ...

  return first.next;
}