Using "fsgets" in C How can Split string and make a structure

Question

i want to read from the file using fgets and send the information to struct in array so i write this code but the output in incorrect

this is the file i tried to read

1#18042022#14:30#Birzeit#Ramallah#6#15
2#18042022#11:45#Birzeit#Birzeit#6#1
13#19042022#14:30#Birzeit#Atara#6#20
53#20042022#14:00#Birzeit#Nablus#6#7

I have written the following code in attempt to fill an array of struct bus:

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

typedef struct bus
{
    int busnumber;
    char* traveldate;
    char* traveltime;
    char* fromdestination;
    char* todestination;
    float price;
    int capacity;

}bus;

int main()
{
    bus busses[numberofbuses("busses.txt")];
    FILE *fp;
    fp = fopen("busses.txt" , "r");
    char line_data[10240];
    int i = 0;
    char c;
   while(fgets(line_data,10240, fp))
    {
        printf("%s\n",line_data);
    busses[i].busnumber = atoi(strtok(line_data,"#"));
    busses[i].traveldate = strtok(NULL,"#");
    busses[i].traveltime = strtok(NULL,"#");
    busses[i].fromdestination = strtok(NULL,"#");
    busses[i].todestination = strtok(NULL,"#");
    busses[i].price = atof(strtok(NULL,"#"));
    busses[i].capacity = atoi(strtok(NULL,"#"));
    i++;
    }
printf("%s\n",busses[0].traveldate);
    for (int j = 0 ; j < numberofbuses("busses.txt") ; j++)
    {
        printf("%d %s %s %s %s %f %d\n", busses[j].busnumber,busses[j].traveldate,busses[j].traveltime,busses[j].fromdestination,busses[j].todestination , busses[j].price, busses[j].capacity);
    }

   fclose(fp);
   return(0);
}
int numberofbuses (char *filename)
{
    FILE *fp;
    char c;
    int count = 0;
    fp = fopen(filename,"r");
    if(fp == NULL)
    {
        perror("Error opening file");
        return(-1);
    }
    else
    {
        for (c = getc(fp); c != EOF; c = getc(fp))
            if (c == '\n')
                count = count + 1;
        return count;
    }
}

Answer 1

You are making things a bit difficult on yourself and mixing a few concepts. When you declare your struct bus and typedef, you declare pointers for each string you want to read. These pointers are uninitialized and must have storage allocated for each before attempting to copy a string to the memory location. (always answer the question "To what valid memory address does my pointer point?" for every pointer you use)

While allocating for each string can be done relatively simply, it will add unneeded complexity to your code and is unnecessary here. If your values are as shown in your sample data, then you can simply declare fixed arrays instead of pointers to the data you want to store. Looking, the following array sizes will be sufficient for your data (with roughly 50% more storage than required, and room for the nul-terminating character), e.g.

    traveldate        16-bytes
    traveltime         8-bytes
    fromdestination   63-bytes
    todestination     63-bytes

(adjust as needed)

You can declare a few constants to set the size of each array, and provide a single convenient location at the top of your code where you can make a change if required. The constants and your re-written struct could be:

#define MAXC 1024   /* if you need a constant, #define one (or more) */
#define NBUS  128
#define DESTC  64
#define DATEC  16
#define TIMEC   8

typedef struct bus {
  int busnumber;
  char traveldate[DATEC];       /* use fixed arrays or allocate for each */
  char traveltime[TIMEC];
  char fromdestination[DESTC];
  char todestination[DESTC];
  float price;
  int capacity;
} bus;

You are thinking correctly in reading an entire line at a time. MAXC above sets the maximum number of characters for your read-buffer to ensure a line is fully consumed with each call to fgets(). The NBUS constant sets the max number of busses to be read (the array size for your busses array).

Your alternative is to dynamically allocate storage for a block of memory to hold the busses. That also is relatively easy, and it allows you to grow the block of memory to handle however many busses are in your file without needed to know "how many" beforehand. (NEVER make two passes (full-reads) of your data file just to find out how many lines there are -- highly inefficient) However, for the parsing example, we will use a fixed array limited to holding NBUS busses.

With that in mind you can declare your busses array and read each line into a buffer, taking the filename to read as the first command-line argument to your program (or by default reading from stdin if no argument is given) like:

int main (int argc, char **argv) {
  
  char buf[MAXC] = "";                        /* read buffer */
  bus busses[NBUS] = {{ .busnumber = 0 }};    /* array of bus */
  size_t n = 0;                               /* number of busses */
  /* use filename provided as 1st argument (stdin by default) */
  FILE *fp = argc > 1 ? fopen (argv[1], "r") : stdin;

  if (!fp) {  /* validate file open for reading */
      perror ("file open failed");
      return 1;
  }
  
  /* while array not full, read line */
  while (n < NBUS && fgets (buf, MAXC, fp)) {
    ...

While there are several alternatives for separating the lines on '#' delimiters, for your fixed-data, by far the simplest is using sscanf() and a carefully crafted format string. You can separate the data while protecting your array bounds with the following:

    "%d#%15[^#]#%7[^#]#%63[^#]#%63[^#]#%f#%d"

The sscanf() format string above will separate each line into:

• an integer, followed by '#',
• a string of no more than 15 characters followed by a '#',
• a string of no more than 7 characters followed by a '#',
• a string of no more than 63 characters followed by a '#',
• a string of no more than 63 characters followed by a '#',
• a float value followed by a '#', and finally
• an integer.

You count the number of conversions anticipated (7 above) and you validate the return of sscanf() with that value. You can use a counter variable (n here) to track the number of busses read, and only increment n in your read-loop upon a successful separation of all values into your array of struct. You protect the busses array bounds by checking n against NBUS as part of the condition of your read loop (shown above).

Putting it altogether, your read and separation loop to read your file and fill your busses array will look like:

  /* while array not full, read line */
  while (n < NBUS && fgets (buf, MAXC, fp)) {
    /* separate line into stuct variables - VALIDATE return */
    if (sscanf (buf, "%d#%15[^#]#%7[^#]#%63[^#]#%63[^#]#%f#%d", 
                &busses[n].busnumber, busses[n].traveldate,
                busses[n].traveltime, busses[n].fromdestination,
                busses[n].todestination, &busses[n].price,
                &busses[n].capacity) == 7) {
      n++;  /* increment count only on successful separation */
    }
  }

That's all there is to it. You can now do whatever you like with your busses array.

Putting it together into a full example, you would have:

#include <stdio.h>

#define MAXC 1024   /* if you need a constant, #define one (or more) */
#define NBUS  128
#define DESTC  64
#define DATEC  16
#define TIMEC   8

typedef struct bus {
  int busnumber;
  char traveldate[DATEC];       /* use fixed arrays or allocate for each */
  char traveltime[TIMEC];
  char fromdestination[DESTC];
  char todestination[DESTC];
  float price;
  int capacity;
} bus;

int main (int argc, char **argv) {
  
  char buf[MAXC] = "";                        /* read buffer */
  bus busses[NBUS] = {{ .busnumber = 0 }};    /* array of bus */
  size_t n = 0;                               /* number of busses */
  /* use filename provided as 1st argument (stdin by default) */
  FILE *fp = argc > 1 ? fopen (argv[1], "r") : stdin;

  if (!fp) {  /* validate file open for reading */
      perror ("file open failed");
      return 1;
  }
  
  /* while array not full, read line */
  while (n < NBUS && fgets (buf, MAXC, fp)) {
    /* separate line into stuct variables - VALIDATE return */
    if (sscanf (buf, "%d#%15[^#]#%7[^#]#%63[^#]#%63[^#]#%f#%d", 
                &busses[n].busnumber, busses[n].traveldate,
                busses[n].traveltime, busses[n].fromdestination,
                busses[n].todestination, &busses[n].price,
                &busses[n].capacity) == 7) {
      n++;  /* increment count only on successful separation */
    }
  }

  if (fp != stdin)   /* close file if not stdin */
      fclose (fp);
  
  for (size_t i = 0; i < n; i++) {  /* output results */
    printf ("%4d  %s  %s  %-12s  %-12s  %6.2f  %3d\n",
            busses[i].busnumber, busses[i].traveldate,
            busses[i].traveltime, busses[i].fromdestination,
            busses[i].todestination, busses[i].price,
            busses[i].capacity);
  }
}

Example Use/Output

With your sample data in the file dat/busses.txt you would get the following upon running the program:

./bin/readbusses dat/busses.txt
   1  18042022  14:30  Birzeit       Ramallah        6.00   15
   2  18042022  11:45  Birzeit       Birzeit         6.00    1
  13  19042022  14:30  Birzeit       Atara           6.00   20
  53  20042022  14:00  Birzeit       Nablus          6.00    7

There are many different ways to approach separating each line, but from a simplicity standpoint, and a robust standpoint (an error in any one line of data only effects data from that line and doesn't break the read from that point forward, such as if you attempted to use fscanf() directly on the file -- without clearing to end of line manually on error.)

Look things over and let me know if you have further questions.

Answer 2

It is a cool idea to read the entire file as a single string to be tokenized, but it does add a little complexity to reading it. Things work just as nicely with the strdup() option.

User input is always a total pain, but in your case it can be done fairly simply. You have to build the proper tools though:

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


typedef struct bus
{
    int    busnumber;
    char * traveldate;
    char * traveltime;
    char * fromdestination;
    char * todestination;
    float  price;
    int    capacity;
} bus;


bool read_string( FILE * f, char ** s )
{
    char buf[100] = {0};
    if (fscanf( f, "%99[^#]", buf ) != 1) return false;
    return (*s = strdup( buf ));
}
bool read_int      ( FILE * f, int   * n      ) { return fscanf( f, "%d", n ) == 1; }
bool read_float    ( FILE * f, float * n      ) { return fscanf( f, "%f", n ) == 1; }
bool read_separator( FILE * f, char separator ) { return fgetc( f ) == separator; }


bool read_bus( FILE * f, bus * bus )
{
    return  read_int   ( f, &bus->busnumber       ) and read_separator( f, '#'  )
        and read_string( f, &bus->traveldate      ) and read_separator( f, '#'  )
        and read_string( f, &bus->traveltime      ) and read_separator( f, '#'  )
        and read_string( f, &bus->fromdestination ) and read_separator( f, '#'  )
        and read_string( f, &bus->todestination   ) and read_separator( f, '#'  )
        and read_float ( f, &bus->price           ) and read_separator( f, '#'  )
        and read_int   ( f, &bus->capacity        ) and read_separator( f, '\n' );
}


bool load_bus_schedule( const char * filename, bus busses[], int * num_busses, int MAX_BUSSES )
{
    FILE * f = fopen( filename, "r" );
    if (!f) return false;

    while ((*num_busses < MAX_BUSSES) and read_bus( f, &busses[ *num_busses ] ))
      *num_busses += 1;

    bool ok = feof( f );
    fclose( f );
    return *num_busses and ok;
}


void free_bus( bus * bus )
{
    free( bus->traveldate      );    bus->traveldate      = NULL;
    free( bus->traveltime      );    bus->traveltime      = NULL;
    free( bus->fromdestination );    bus->fromdestination = NULL;
    free( bus->todestination   );    bus->todestination   = NULL;
}


int main()
{
    enum { MAX_BUSSES = 100 };
    bus busses[ MAX_BUSSES ] = {{0}};
    int num_busses = 0;

    if (!load_bus_schedule( "busses.txt", busses, &num_busses, MAX_BUSSES ))
    {
      fprintf( stderr, "Could not read bus schedule.\n" );
      return EXIT_FAILURE;
    }

    printf( "number of busses = %d\n", num_busses );
    for (int n = 0;  n < num_busses;  n += 1)
        printf(
            "bus %-2d  from %-8s  to %-8s  on %8s  at %5s  price %5g?  has %d seat%s available\n",
            busses[n].busnumber,
            busses[n].fromdestination,
            busses[n].todestination,
            busses[n].traveldate,
            busses[n].traveltime,
            busses[n].price,
            busses[n].capacity,
            busses[n].capacity == 1 ? "" : "s"
        );
        
    while (num_busses) free_bus( &busses[--num_busses] );
    return EXIT_SUCCESS;
}

Oh, from your OP I surmize that the source file is a CSV format with # as field separators and \n as record separators.

1#18042022#14:30#Birzeit#Ramallah#6#15
2#18042022#11:45#Birzeit#Birzeit#7.5#1
13#19042022#14:30#Birzeit#Atara#8#20
53#20042022#14:00#Birzeit#Nablus#10.5#7

Output:

number of busses = 4
bus 1   from Birzeit   to Ramallah  on 18042022  at 14:30  price     6?  has 15 seats available
bus 2   from Birzeit   to Birzeit   on 18042022  at 11:45  price   7.5?  has 1 seat available
bus 13  from Birzeit   to Atara     on 19042022  at 14:30  price     8?  has 20 seats available
bus 53  from Birzeit   to Nablus    on 20042022  at 14:00  price  10.5?  has 7 seats available

A point of note: David C. Rankin’s answer recommends that you first read each line of data and then tokenize it into fields. This is good advice (and my usual recommendation as well).

The approach used here is to simply fail on the first error, for the entire file. It is possible to add stuff that will recover properly, but the choice of whether to do that or not depends on how you intend to deal with the failure. This approach requires input to be correct and does not allow for error. This is a valid approach.

The choice of approach will ultimately depend on human factors (such as business considerations). For a schoolwork I think that rejecting all bad input is a valid requirement.