How to read line by line some file in C with fscanf() without getting a phantom line?

Question

I have a function in my code that reads some file line by line and creates structures from them. A professor said to me that there can be a problem and mentioned phantom line. Can someone see my function and explain me where is the problem?

This is my code:

void readComponentList(ComponentList *cl, char *fileName)
{
   FILE *file = fopen(fileName, "r");
   if (file == NULL) { perror(fileName);  exit(1); } // If the file doesn't exist, the program termitates with exit code 1
   int r;
   Component *c = newComponent(cl);
   // Creates useful component c by inserting line informations as arguments in the structure
   r = fscanf(file, "%24s %24s %24s %d %lf", c->type, c->model, c->unit, &(c->weight), &(c->price));
   while (r != EOF) // Doing the same thing for the rest of the lines
   {
      c = newComponent(cl);
      r = fscanf(file, "%24s %24s %24s %d %lf", c->type, c->model, c->unit, &(c->weight), &(c->price);
      // Since EOF only occurs after an unsuccessful read attempt, an additional "phantom line" with undefined content is read in here, which could lead to crashes.

   }
   fclose(file);
}

This is the file example that I am reading:

Motor M5x40 Stk 5 0.05
Elevator M5x60 Stk 6 0.05
ACM L-H-100 Stk 1250 530
SSM L-100 Stk 0 0
ElevatorW W3 Stk 0 0
Metal- kg 1000 344200

Component and ComponentList structures:

typedef struct
{
   char     type[25];
   char     model[25];
   char     unit[25];
   int      weight;
   double   price;
   StepList *construction_steps;
} Component;

typedef struct
{
   Component **components;
   int count;
   int allocated;
} ComponentList;

What do you do immediately after your call to `fscanf()` if it returns `EOF`? — Andrew Henle, Jan 21 '20 at 16:33
@AndrewHenle, can I then delete the first call of fscanf() and use do-while? — , Jan 21 '20 at 16:36
From the code you posted, no, it's not that easy. You'll need to do some reordering too. — Andrew Henle, Jan 21 '20 at 16:38
If the `fscanf()` in the loop body fails, you have no data in the `c` component, but it is part of your component list. That's not good — it's the phantom row that was discussed. — Jonathan Leffler, Jan 21 '20 at 16:41
For even better error handling I recommend you [read whole lines](https://en.cppreference.com/w/c/io/fgets) and then use `sscanf` to parse the line. — Some programmer dude, Jan 21 '20 at 17:43

Support Ukraine · Answer 1 · 2020-01-21T20:05:03.553

It's not clear from the posted code but I assume the line

Component *c = newComponent(cl);

will 1) Create a new Component element and 2) Inserted it into the list cl

So when you do it before calling fscanf the element is already inserted **even if fscanf fails. Consequently, you'll end up with an "empty" element in the list (aka a phantom element).

To avoid that you need to move the line: Component *c = newComponent(cl); so that it is only called after a successful scanf.

Perhaps something like:

Component tmp;  // Scan into a local tmp variable
while (1)
{
   r = fscanf(file, "%24s %24s %24s %d %lf", 
                    tmp.type, tmp.model, tmp.unit, &tmp.weight, &tmp.price;

   if (r == EOF) break;

   if (r == 5)
   {
       // A complete struct was read so put it into the list
       c = newComponent(cl);
       *c = tmp;   // Copy the tmp struct into the new element in the list
   }
   else
   {
       // Didn't get the expected number of vars scanned.
       // Add error handling ....
   }
}

EDIT

As OP is concerned about the line Component tmp;, this is an alternative approach:

char     type[25];
char     model[25];
char     unit[25];
int      weight;
double   price;

while (1)
{
   r = fscanf(file, "%24s %24s %24s %d %lf", 
                    type, model, unit, &weight, &price;

   if (r == EOF) break;

   if (r == 5)
   {
       // A complete struct was read so put it into the list
       c = newComponent(cl);
       strcpy(c->type, type);
       strcpy(c->model, model);
       strcpy(c->unit, unit);
       c->weight = weight;
       c->price = price;
   }
   else
   {
       // Didn't get the expected number of vars scanned.
       // Add error handling ....
   }
}

The problem with that code is that I have a list inside Component structure that will be created by calling the method newComponent() and when creating tmp, that list will be some random pointer:) — , Jan 21 '20 at 17:06
@Delprof Assuming that `Component` is an ordinary struct, I don't see any problem. Creating a local instance is OK. — Support Ukraine, Jan 21 '20 at 17:15
In the first line, by creating tmp, there will be no allocated space for the list inside the structure. If I want to add something inside the list, the error will occur. I added the code for structures :) — , Jan 21 '20 at 17:22
@Delprof By creating tmp you create local storage for doing the scan. Then - if the scan is successful - you create the element in the list and copy from tmp to the element in the list. That's perfectly OK. Did you try it out? — Support Ukraine, Jan 21 '20 at 19:47
@Delprof Since your concerned about the local `Component tmp` I posted an alternative for you. — Support Ukraine, Jan 21 '20 at 19:57

How to read line by line some file in C with fscanf() without getting a phantom line?

1 Answers1