0

I have the following enum and struct:

enum Destination { unknown = 0, hosok, parlament, var };

struct Client
{
  char name[30];
  char email[30];
  char phone_num[11];
  int client_num;
  enum Destination destination;
  struct tm registration_date;
};

When I invoke the following method, it reads the first struct and print it's name, then I get a segmentation fault.

void list_clientss()
{
  FILE *f = fopen(filename, "r");
  if( f == NULL )
  {
    perror("Error");
  }
  struct Client client;
  while( !feof( f ) )
  {
    fread(&client, sizeof(struct Client), sizeof(struct Client), f);
    printf("Name: %s\n", client.name);
  }
  fclose(f);
}

What did I wrong?

Gene
  • 46,253
  • 4
  • 58
  • 96
prophet4955
  • 5
  • 3
  • 4
    Possible duplicate of [Why is “while ( !feof (file) )” always wrong?](https://stackoverflow.com/questions/5431941/why-is-while-feof-file-always-wrong) – Bo Persson Oct 28 '17 at 16:13
  • 2
    Are you sure it's segfaulting in `feof`? Your `fread` call is wrong and you're trashing your stack. You should be calling `fread(&client, sizeof(struct Client), 1, f);` – Cornstalks Oct 28 '17 at 16:15
  • You're ready _1_ record of type `struct Client`. Say `fread(&client, sizeof(struct Client), 1, f);` – Gene Oct 28 '17 at 16:15
  • when your f is NULL you still trying to use it – Artemy Vysotsky Oct 28 '17 at 16:17
  • @Cornstalks You're right, my fread call was wrong. But now it prints the last record twice.. – prophet4955 Oct 28 '17 at 16:38
  • 1
    @MartinMagyar Why is “while ( !feof (file) )” always wrong? – Martin James Oct 28 '17 at 16:58
  • 1
    @MartinMagyar -- the last record is being printed twice because [you should never use `while (!feof(fp)) {}`](https://stackoverflow.com/questions/5431941/why-is-while-feof-file-always-wrong). – ad absurdum Oct 28 '17 at 16:59
  • Not that it's happening in your case but in general if you try to fopen() a file and it fails the program shouldn't just give a message and continue. Use an exit(1) or something in this case. And feof() on a file that didn't open wouldn't be pretty, might not give a segfault. That's true of reading from a file that didn't open too, I've never used fread(). – Alan Corey Oct 29 '17 at 02:54
  • Oh, the duplicate line. Try another test for feof( ) before you actually use the data you read. The while loop may set it but not bail out in time. And I use !feof() fairly often, nobody told me I shouldn't. – Alan Corey Oct 29 '17 at 03:02
  • @AlanCorey The have now:) The 'Why is “while ( !feof (file) )” always wrong?' Q&A, as linked by Bo, is the second question down in the C 'frequent' list. – Martin James Oct 30 '17 at 10:41
  • Works fine as long as you test it again inside the loop before you actually use the data. I usually and it with an strlen() check on what I read. Being able to read a file until EOF is pretty basic. – Alan Corey Oct 31 '17 at 19:21

1 Answers1

2

Firstly, your fread calls should be as follows:

fread(&client, sizeof(struct Client), 1, f);

Secondly, instead of using feof, you might use the return value of fread. fread returns the number of elements to read that you have passed to it. You can check if this number is different than one. For example,

while (fread(&client, sizeof(struct Client), 1, f) == 1) {
    printf("Name: %s\n", client.name);
}

Edit 1: Update the while loop to more idiomatic and elegant version as Weather Vane suggested.

eozd
  • 1,153
  • 1
  • 6
  • 15