1

I'm trying to allocate memory for a new struct for each line in a file, but when the file is empty, my loop still allocates memory once. the issues is using while(!feof(file)), but I can't figure out an alternative check for the while loop.

the loop looks like this:

while(!feof(f))
{
    p = (struct PlayerTime*)malloc(sizeof(struct PlayerTime));
    head = p;
    fscanf(f, "%f %s", &p->seconds, p->name);
    p = p->next;
}

the pointers and structs are all defined prior to the loop, I just can't figure out how to get this to not loop if there's nothing in the file.

  • In addition to the link above, see the comments to the accepted answer [here](http://stackoverflow.com/a/11280535/62576). – Ken White Oct 05 '13 at 23:00

2 Answers2

0
  1. feof(f) says that the EOF hasn't been hit yet
  2. fscanf hits the EOF and fails
  3. feof(f) stops the loop, because EOF has been hit

Correct approach:

while (fscanf(f, "%f %s", &p->seconds, p->name) == 2) {
    ...
}

Hint: Also spend more time thinking when and how should the memory be allocated, what are scenarios that might happen and how they should be handled.

LihO
  • 41,190
  • 11
  • 99
  • 167
0

This has been discussed ad nauseam; feof doesn't tell you if the file is going to finish at the next read, but if a read has been tried and failed due to end of file.

In your case, a solution can be to check if the read failed (by checking the return value of fscanf), and in that case to deallocate the structure; this also makes your code more robust, since it checks also for errors others than EOF (e.g. IO errors, invalid data format, ...).

Incidentally, p = p->next won't do what you expect. If you are building a linked list "on the fly", you could do something like this:

// Allocate the space for the first element
struct PlayerTime *head=malloc(sizeof(*head));
// p will always point to a pointer to the element to be filled;
// let's start with the head
struct PlayerTime **p=&head;
// Try to read
while(fscanf(f, "%f %s", &((*p)->seconds), (*p)->name)==2)
{
    // If we are here, the last read was successful
    // Move p to the pointer to the next element
    p = &((*p)->next);
    // ... and allocate the space for such element
    *p = malloc(sizeof(**p));
}
// After exit, there's an extra element that we allocated but we couldn't read
// Free it
free(*p);
// And put the relevant pointer to NULL
// (it will terminate the list, or set head to NULL if no element has been read)
*p=NULL;
Matteo Italia
  • 123,740
  • 17
  • 206
  • 299