Solved

Buffer problem while reading from a file

Posted on 2004-10-25
242 Views
Last Modified: 2010-08-05
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
Question by:The_Kingpin08
    12 Comments
     
    LVL 45

    Assisted Solution

    by:Kdo
    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
     

    Author Comment

    by:The_Kingpin08
    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
     
    LVL 3

    Accepted Solution

    by:
    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
     

    Author Comment

    by:The_Kingpin08
    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
     

    Author Comment

    by:The_Kingpin08
    The "num_books++;" seems to be the problem since when I take it out, everything works fine...
    0
     
    LVL 3

    Expert Comment

    by:Indrawati
    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
     

    Author Comment

    by:The_Kingpin08
    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
     
    LVL 3

    Expert Comment

    by:Indrawati
    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
     

    Author Comment

    by:The_Kingpin08
    each line contains a word, two max. Even when I increase the buffer' size the error still appears after the first struct...
    0
     
    LVL 3

    Expert Comment

    by:Indrawati
    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
     

    Author Comment

    by:The_Kingpin08
    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
     
    LVL 23

    Expert Comment

    by:brettmjohnson
    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

    Write Comment

    Please enter a first name

    Please enter a last name

    We will never share this with anyone. Privacy Policy Terms of Use

    Featured Post

    What Should I Do With This Threat Intelligence?

    Are you wondering if you actually need threat intelligence? The answer is yes. We explain the basics for creating useful threat intelligence.

    An Outlet in Cocoa is a persistent reference to a GUI control; it connects a property (a variable) to a control.  For example, it is common to create an Outlet for the text field GUI control and change the text that appears in this field via that Ou…
    This is a short and sweet, but (hopefully) to the point article. There seems to be some fundamental misunderstanding about the function prototype for the "main" function in C and C++, more specifically what type this function should return. I see so…
    Video by: Grant
    The goal of this video is to provide viewers with basic examples to understand and use nested-loops in the C programming language.
    The goal of this video is to provide viewers with basic examples to understand how to create, access, and change arrays in the C programming language.

    875 members asked questions and received personalized solutions in the past 7 days.

    Join the community of 500,000 technology professionals and ask your questions.

    Join & Ask a Question

    Need Help in Real-Time?

    Connect with top rated Experts

    13 Experts available now in Live!

    Get 1:1 Help Now