0

I need some help with an assignement I have to do for school which consists in sorting some books after the title,author and the publication date of it. All the infos are given as a string in a txt file using a delimiter between them. The problem is that I don't manage to properly read the data, my program crashes after I try to perform a strcpy(). Can you guys help me with this and tell me what have I done wrong?

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

struct book
{
    char author[100],title[100];
    int year;
};

int main()
{
    struct book b[25];
    int i,n;
    char intro[25][150],*p;
    const char delim[2] = "#";
    FILE *fp;
    fp=fopen("text.txt", "r");
    fscanf(fp,"%d",&n);
    for(i=0;i<=n;i++)
        {
            fgets(intro[i], sizeof (intro[i]), fp);
            p=strtok(intro[i], delim);
            strcpy(b[i].title,p);
            p=strtok(NULL, delim);
            strcpy(b[i].author,p); /// The program works until it reaches this point - after performing this strcpy() it crashes 
            if(p!=NULL)
            {
                p=strtok(NULL,delim);
                b[i].year=atoi(p);

            }


        }
return 0;
}

An example of input could be this:

5
Lord Of The Rings#JRR Tolkien#2003
Emotional Intelligence#Daniel Goleman#1977
Harry Potter#JK Rowling#1997
The Foundation#Isaac Asimov#1952
Dune#Frank Herbert#1965
  • when calling `fopen()` always check (!=NULL) the returned value to assure the operation was successful. – user3629249 Dec 06 '15 at 09:14
  • in general, the code cannot assume that a input file contains a specific number of lines. Suggest i=0; while( i<25 && fgets(intro[i], sizeof (intro[i]), fp) )` so the loop is controlled by the max number of iterations and by successfully reading a line from the file. – user3629249 Dec 06 '15 at 09:17
  • there are 'magic' numbers in the code. 'magic' numbers make the code harder to understand, debug, maintain. The `magic` numbers are 25, 150. Suggest either using #define statements or an enum to give those numbers meaningful names, then use those meaningful names throughout the code. – user3629249 Dec 06 '15 at 09:19
  • strongly suggest reading the 'n' value first, then define the number of rows in the arrays using that 'n' (this 'may' mean using `malloc()` rather than defining the arrays on the stack.) – user3629249 Dec 06 '15 at 09:21
  • always check the returned value from `fgets()` and from `fscanf()` and from `strtok()` to assure the operation was successful – user3629249 Dec 06 '15 at 09:23
  • the call to `fscanf()` does not consume a trailing . Suggest calling `getchar()` in a loop, immediately after the call to `fscanf()`, until the input character (actually an int) is '\n'. – user3629249 Dec 06 '15 at 09:25
  • this kind of line: `if(p!=NULL)` should be after every call to `strtok()`, not just after the second call to `strtok()` – user3629249 Dec 06 '15 at 09:27

1 Answers1

0

The problem is with the newline left in the file after the initial fscanf() call.

This

fscanf(fp,"%d",&n);

reads the 5 and the subsequent fgets() reads just the \n. So this is not what you want. Read n using fgets() and convert it into integer using sscanf() or strto*. For example, instead of the fscanf() call, you can do:

char str[256];

fgets(str, sizeof str, fp);
sscanf(str, "%d", &n);

to read the n from file.

You should also check if strtok() returns NULL. If you did, you would have figured out the issue easily.

Also, your need to go from 0 to n-1. So the condition in the for loop is wrong. It should be

for(i=0; i<n; i++)
P.P
  • 117,907
  • 20
  • 175
  • 238