[Webinar] Streamline your web hosting managementRegister Today

x
  • Status: Solved
  • Priority: Medium
  • Security: Public
  • Views: 300
  • Last Modified:

Buffer problem while reading from a file

Hi all,

I need to read every line from a file and split them in groups of 6 to form struct. Every struct represents a book containing it's information. Once the struct is created, I must add it to an array of struct, containing every "book".

Now I can read each line of my file and separate them in groups of 6 line to create struct, but somehow my buffer is confused and I get only half-word  !?

I used a switch to manage the linformation I get from the file depending on it's line_number. This way a line #1 is added to the record #1 of a struct.

I also think I have a problem with my array books which is supposed to contain every struck (book or group of 6 lines) because when I use books(value) it doesn't return me what it is supposed to... Thanks for all your help guys, I wish I can finish this by tonight :(

 #include "stdafx.h"
 #include "stdio.h"
 #include <stdlib.h>
 
 /* Struct containing the informations on a book */
 struct book
 {
     char title[80];
     char autor[40];
     char editor[20];
     char ISBN[10];
     char subject[20];
     char releaseDate[10];
 };
 typedef struct book bookInfo;


  /* Function that reads the current line of the file and creates struct */
int parseline(bookInfo *out, const char *str, int line_num)
{
  int i;

  switch(line_num)
      {
      case 0:
                  /* TITLE RECORD */
                  /* stop on tab, but also on end of string or overflow so we don't crash */
                  for(i=0; str[i] != '\t' && str[i] != '\n' && i < 63;i++)
                        out->title[i] = str[i];

                        /* return -1 on parse error */
                        if(str[i] != '\t')
                        return -1;

                        /* for convenience, move the pointer along for the next field */
                        str += i +1;
                  break;

      case 1:
                  /* AUTHOR RECORD */
                  /* stop on tab, but also on end of string or overflow so we don't crash */
                  for(i=0; str[i] != '\t' && str[i] != '\n' && i < 63;i++)
                        out->autor[i] = str[i];

                        /* return -1 on parse error */
                        if(str[i] != '\t')
                        return -1;

                        /* for convenience, move the pointer along for the next field */
                        str += i +1;
                        break;

      case 2:
                  /* EDITOR RECORD */
                  /* stop on tab, but also on end of string or overflow so we don't crash */
                  for(i=0; str[i] != '\t' && str[i] != '\n' && i < 63;i++)
                        out->editor[i] = str[i];

                        /* return -1 on parse error */
                        if(str[i] != '\t')
                        return -1;

                        /* for convenience, move the pointer along for the next field */
                        str += i +1;
                        break;

      case 3:
                  /* ISBN RECORD */
                  /* stop on tab, but also on end of string or overflow so we don't crash */
                  for(i=0; str[i] != '\t' && str[i] != '\n' && i < 63;i++)
                        out->ISBN[i] = str[i];

                        /* return -1 on parse error */
                        if(str[i] != '\t')
                        return -1;

                        /* for convenience, move the pointer along for the next field */
                        str += i +1;
                        break;

      case 4:
                  /* SUBJECT RECORD */
                  /* stop on tab, but also on end of string or overflow so we don't crash */
                  for(i=0; str[i] != '\t' && str[i] != '\n' && i < 63;i++)
                        out->subject[i] = str[i];

                        /* return -1 on parse error */
                        if(str[i] != '\t')
                        return -1;

                        /* for convenience, move the pointer along for the next field */
                        str += i +1;
                        break;

      case 5:
                  /* releaseDate RECORD */
                  /* stop on tab, but also on end of string or overflow so we don't crash */
                  for(i=0; str[i] != '\t' && str[i] != '\n' && i < 63;i++)
                        out->releaseDate[i] = str[i];

                        /* return -1 on parse error */
                        if(str[i] != '\t')
                        return -1;

                        /* for convenience, move the pointer along for the next field */
                        str += i +1;
                        break;
      }
  /* 0 to indicate OK */
  return 0;
}
 
 
 int main()
 {
   FILE *file;                 // File containing the data
   bookInfo *books = NULL;     // Array of books, UNKNOWN size
   char buff[1000];         // Buffer to read in the file
   int i;
   int num_books = 0;          // Keep the count of number read in
   int line_number = 0;        /* Used to count the 6 lines required
                                             to create a new struct */
 
   /* First step we open the file and see if it's correct */
   file = fopen("livres.txt", "r");
   if(file==NULL)
      {
      printf("Error: can't open file.\n");
      return 1;
      }
   else
      {
        /* First loop tthat goes through every line of the file */
      while(fgets(buff, sizeof(buff), file) != NULL)
        {
                  bookInfo BOOK;
                  /* Resize the array containing the structs */
                  void *temp = realloc( books, (num_books + 1) * sizeof(bookInfo));
                  if ( temp != NULL )
                    {
                      books = temp;
                        
                      /* Creation of the struct */
                      if(parseline(&BOOK, buff, line_number) == 0)
                          {
                          books = realloc(books, (num_books + 1) * sizeof(bookInfo));
                          }
                     }
                  
                  if(line_number==6)
                        {
                        books[num_books] = BOOK; /* We add the new book (bookInfo) in the array of struct
                                                                  at the correct position only when all the fields of the
                                                                  struct are full */

                        num_books++; // Increase the number of line read
                        line_number=0; // Reset the line number so we can create a new struct
                        }

                  line_number++;
                  fgets(buff, sizeof(buff+1), file);
             }
        } // end else
     
     fclose(file); /* close file */
 
     for(i=0 ; i<num_books; i++)
     {
       printf("%s\t ", books[i].title);
       //printf(every other fields of the struct....)
     }
 
     if(books!=NULL)
     {
       free(books);    /* free any allocated memory */
     }
     return 0;
 }
0
The_Kingpin08
Asked:
The_Kingpin08
2 Solutions
 
Kent OlsenData Warehouse Architect / DBACommented:
Hi The_Kingpin08,

main() can be a lot simpler.

-- The else statement after if(file==NULL) is unnecessary.  If file==NULL main will exit.

-- 32-bit systems won't run out of memory.  consider:  books = (bookinfo *)realloc (books, sizeof(Bookinfo) * Count)

-- It appears that main() is reading the line into buff and calling parseline().  main() then calls fgets() at the end of the loop before it has processed the remainder of the line.  (Lines are tab delimited.  You seem to grab the first parameter from line 1, second from line 2, etc.  Move the fgets() into the if(line_number==6) block.

Good Luck!
Kent
0
 
The_Kingpin08Author Commented:
Hi Kdo,

if the fgets() is placed at the end, it's because I need to read 6 lines to create a struct and before adding it to the array. Also what is the value Count in your realloc function ?

Thanks for the advices, I'll change what I can.

Frank
0
 
IndrawatiCommented:
OK, here goes:

1. in parseline():

      for(i=0; str[i] != '\t' && str[i] != '\n' && i < 63;i++)
            out->title[i] = str[i];

in C, a character string must be terminated with '\0', so you must add the following line right after the previous two lines:

            out->title[i] = '\0';

You must do that for the other members (author, ISBN, etc.) as well.

2. in main():

      /* Resize the array containing the structs */
      void *temp = realloc( books, (num_books + 1) * sizeof(bookInfo));
      if ( temp != NULL )
      {
            books = temp;
                   
            /* Creation of the struct */
            if(parseline(&BOOK, buff, line_number) == 0)
            {
                  books = realloc(books, (num_books + 1) * sizeof(bookInfo));
            }
      }

You don't need all of these, you can delete them all. Instead, you can parse the lines, and realloc only when the line number has reached releaseDate record. Something like:

      parseline(&BOOK, buff, line_number);
      if(line_number==5)
      {
            //realloc here
      }

3. in main():

      if(line_number==6)
      {
            books[num_books] = BOOK; /* We add the new book (bookInfo) in the array of struct at the correct position only when all the fields of the struct are full */

            num_books++; // Increase the number of line read
            line_number=0; // Reset the line number so we can create a new struct
      }

      line_number++;
      fgets(buff, sizeof(buff+1), file);

OK, first, the equality condition should be if(line number == 5) instead of == 6 (you count from 0 to 5).
Second, you should realloc inside the compound statement following the if statement.
Third, you don't need the fgets(buff, sizeof(buff+1), file); line anymore, since the statements are already inside a while(fgets(...)) statement.
Fourth, you should not increment line_number when the line number is equal to 5, since line_number is already reset to 0 inside the if statement.

So you get:

      if(line_number==5)
      {
            num_books++; // Increase the number of line read
            void *temp = realloc( books, (num_books + 1) * sizeof(bookInfo));
            if ( temp != NULL )
            {
                  books = (bookInfo*)temp;
            }
            books[num_books-1] = BOOK; /* We add the new book (bookInfo) in the array of struct at the correct position only when all the fields of the struct are full */
            line_number=0; // Reset the line number so we can create a new struct
      }
      else
            line_number++;
0
The new generation of project management tools

With monday.com’s project management tool, you can see what everyone on your team is working in a single glance. Its intuitive dashboards are customizable, so you can create systems that work for you.

 
The_Kingpin08Author Commented:
Thanks Indrawati, I'm pretty sure your solution will work.

Still when compiling I get an error I didn't had before...

The line "void *temp = realloc( books, (num_books + 1) * sizeof(bookInfo));" ---> syntax error : missing ';' before 'type'
The line  "if ( temp != NULL )" ---> 'temp' : undeclared identifier & (warning) 'int' differs in levels of indirection from 'void *'
and finally "books = (bookInfo*)temp;" ---> 'type cast' : conversion from 'int' to 'bookInfo *' of greater size

Now I'm really lost with these error, the temp declaration is the same and the "if" condition too !!!

Thanks again, Frank

0
 
The_Kingpin08Author Commented:
The "num_books++;" seems to be the problem since when I take it out, everything works fine...
0
 
IndrawatiCommented:
It's very hard to tell where the problem is without the complete code. Anyway, here's my main() function, you can see where the difference lies.

 int main()
 {
   FILE *file;                 // File containing the data
   bookInfo *books = NULL;     // Array of books, UNKNOWN size
   char buff[1000];         // Buffer to read in the file
   int i;
   int num_books = 0;          // Keep the count of number read in
   int line_number = 0;        /* Used to count the 6 lines required
                                      to create a new struct */
 
   /* First step we open the file and see if it's correct */
   file = fopen("livres.txt", "r");
   if(file==NULL)
      {
      printf("Error: can't open file.\n");
      return 1;
      }
   else
      {
       /* First loop tthat goes through every line of the file */
      while(fgets(buff, sizeof(buff)-1, file) != NULL)
        {
               bookInfo BOOK;
               parseline(&BOOK, buff, line_number);
               if(line_number==5)
                  {
                           num_books++; // Increase the number of line read
                              void *temp = realloc( books, (num_books + 1) * sizeof(bookInfo));
                              if ( temp != NULL )
                              {
                                 books = (bookInfo*)temp;
                              }
                    books[num_books-1] = BOOK; /* We add the new book (bookInfo) in the array of struct
                                                       at the correct position only when all the fields of the
                                                       struct are full */
                    line_number=0; // Reset the line number so we can create a new struct
                  }
                     else
                        line_number++;
               //fgets(buff, sizeof(buff)-1, file);
           }
       } // end else
     
     fclose(file); /* close file */
 
     for(i=0 ; i<num_books; i++)
     {
       printf("%s\t ", books[i].title);
       //printf(every other fields of the struct....)
     }
 
     if(books!=NULL)
     {
       free(books);    /* free any allocated memory */
     }
     return 0;
 }
0
 
The_Kingpin08Author Commented:
Damn no edit button ?

Anyway thanks a lot Indrawati, it worked pretty well once I change the num_books++;

However, when the first struct is finished and we go to read line #7 (so we switch to the 2nd struct), the program crashes with a "Access violation writing location" error.
It crashes exactly when we start to read next line with "while(fgets(buff, sizeof(buff), file) != NULL)"

Can it be a problem with the buffer ?

Thanks, Frank
0
 
IndrawatiCommented:
It's possible. Have you checked the content of the text file whether the length of each element does not exceed the length of buff - 1 (999)? Note that fgets reads until a newline is found, it does not stop on tabs.
0
 
The_Kingpin08Author Commented:
each line contains a word, two max. Even when I increase the buffer' size the error still appears after the first struct...
0
 
IndrawatiCommented:
Well as I said earlier, it's very hard to determine the cause of the problem without any codes. Maybe you can post the codes and the content of the text file that crashes?
0
 
The_Kingpin08Author Commented:
ok it works, I just forgot to take out the -1 in books[num_books-1] = BOOK; since I put it after.

Thanks a lot to you guys, your help was very valuable :)
I don't know what I would have done without you, I am really thanksful.

Frank
0
 
brettmjohnsonCommented:
You will notice that the body of each case in the switch statement is
dramatically similar (with the exception of the target string receiving
the value).  This just screams out to have a subroutine parse out a
field, then using strcpy() or strncpy() to copy the value to the target.
Of course, you could write your own routine to scan forward in the
buffer, looking for a delimiter (\t,\n,\0, etc), but it makes more sense
to use the library routines, like strtok(), strsep(), or strcspn().  For
instance:

    case 0:              /* TITLE RECORD */
            if ((i = strcpspn(str, "\t\r\n")) < 1) return -1;
              strncpy(out->title, str, min(i, sizeof(out->title));
              break;


>  bookInfo *books = NULL;     // Array of books, UNKNOWN size
>  ...
>  bookInfo BOOK;
>  ...
>    books[num_books-1] = BOOK; /* We add the new book (bookInfo) in the array of struct
>                                                       at the correct position only when all the fields of the
>                                                       struct are full */

You cannot assign a structure to another structure using =.  In ANSI C, = can only be used
with atomic data types, not complex types such as structures or arrays.  You will need to use
memcpy() instead:

    /* We add the new book (bookInfo) in the array of struct ... */
    memcpy( books[num_books-1], BOOK, sizeof(struct bookInfo));  



>  books = realloc(books, (num_books + 1) * sizeof(bookInfo));

realloc() can be computationally expensive.  Imagine you had 100 books,
or 1000 books, or LoC (Library of Congress) number of books.   Calling
realloc() for each record would be computationally prohibitive.  The experienced
programmer will re-allocate the array in chunks, perhaps a 1000 records at a
time.  This amortizes the reallocs across many records.  A single, reducing
realloc() at the end can rid the array of any excess records that have been
allocated.  This reallocs

   int max_books = 0, num_books = 0;
   ...
   if (num_books >= max_books) {
       if ((books = realloc( books, (max_books += 1000) * sizeof(bookInfo))) == NULL)
             return ENOMEM;
   }
   memcpy(books[num_books++], BOOK, sizeof(bookInfo));

0

Featured Post

Take Control of Web Hosting For Your Clients

As a web developer or IT admin, successfully managing multiple client accounts can be challenging. In this webinar we will look at the tools provided by Media Temple and Plesk to make managing your clients’ hosting easier.

Tackle projects and never again get stuck behind a technical roadblock.
Join Now