0

I'm using the following code to read a binary file and use a struct to output the data. However, I know my data consist of only one record, and it seems to be printing out many records. I'm wondering why this might be?

 FILE *p;
 struct myStruct x;
 p=fopen("myfile","rb");
 //printf("Rcords of file:\n");
 while(1)
 {
     fread(&x,sizeof(x),1,p);
     if(feof(p)!=0)
         break;
        printf("\n\nID:%ld",x.ID);
    }
 fclose(p);
 return 0;

The struct is fairly normal like this:

struct myStruct
{
    int ID;     
    char name[100];
}
Ke.
  • 2,224
  • 5
  • 32
  • 70
  • 1
    `char name;` makes no sense. Names rarely consist of a single char. – πάντα ῥεῖ Sep 17 '16 at 08:15
  • ok ive updated it - happy now? – Ke. Sep 17 '16 at 08:15
  • 1
    How big is the file? What does [`fread`](http://en.cppreference.com/w/c/io/fread) return? – Some programmer dude Sep 17 '16 at 08:17
  • 2
    I'm completely unhappy with your question here, even with your edit. Post a [MCVE] and I may be ready to negotiate. – πάντα ῥεῖ Sep 17 '16 at 08:17
  • I can't do that because its a binary file im opening – Ke. Sep 17 '16 at 08:18
  • @Ke. Regarding `if(feof(p)!=0)` see here please: http://stackoverflow.com/questions/5605125/why-is-iostreameof-inside-a-loop-condition-considered-wrong – πάντα ῥεῖ Sep 17 '16 at 08:19
  • Hi Joachim Pileborg, are you suggesting I check the sizeof the fread return? – Ke. Sep 17 '16 at 08:19
  • Hi πάντα ῥεῖ, yes I'm aware this isnt the best way to do it. This is just test code. I'm trying to figure out why it's printing out so many times, not how to optimise code at this stage. – Ke. Sep 17 '16 at 08:20
  • @Ke I doubt all that is a question of _optimization_. Lookup de-/serialization, and how to do it properly. Your question is of no significant value at Stack Overflow. – πάντα ῥεῖ Sep 17 '16 at 08:23
  • 1
    You can still tell us about the data file: `ls -la myfile` and `wc myfile`. Also what is th return value of `fread()`. How do you expect your program to work correctly if you don't check the results of your functions. – Martin York Sep 17 '16 at 08:23
  • [The `fread` function](http://en.cppreference.com/w/c/io/fread) will, in your case, either return `1`, `0` or `EOF`. Knowing what it returns is good. Also, if [`fread`](http://en.cppreference.com/w/c/io/fread) returns `EOF` and [`ferror`](http://en.cppreference.com/w/c/io/ferror) returns non-zero, then you might need to check [`errno`](http://en.cppreference.com/w/c/error/errno) to see what went wrong. – Some programmer dude Sep 17 '16 at 08:23
  • fread returns 1, the file opens just fine, I'm accessing other data with it. I just dont understand why my data prints out many times, when there is only one record... Noone has been able to explain this yet... – Ke. Sep 17 '16 at 08:25
  • 1
    Does `fread` return `1` *always*? – Some programmer dude Sep 17 '16 at 08:27
  • Hi Joachim yes. The file is fine, it is written by another program, tried and tested. There is nothing wrong with the file. I even know what data it contains. I just cant access it properly. I'm sure I'm doing something wrong in my function as there is only one piece of data for ID and it is returning many when it shouldnt be. – Ke. Sep 17 '16 at 08:35
  • @Ke. What do you mean by "I just cant access it properly.", Surly code `printf("\n\nID:%d",x.ID);` (with the corrected `"%d"`) works. Or are your says some other aspect of access not shown is a problem? – chux - Reinstate Monica Sep 17 '16 at 17:05

2 Answers2

2

Use %d instead of %ld to print an int

And take a look to Why is “while ( !feof (file) )” always wrong?

A struct haves a fixed size, you can use ftell to get the size of the file and then divide using the size of the struct in order to get the number of records, also, always check the result of those functions.

Something like:

FILE *file;
long size;
size_t count, records;

file = fopen("myfile", "rb");
if (file == NULL) {
    perror("fopen");
    return 0;
}
if (fseek(file, 0, SEEK_END) == -1) {
    perror("fseek");
    return 0;
}
size = ftell(file);
if (size == -1) {
    perror("ftell");
    return 0;
}
if (fseek(file, 0, SEEK_SET) == -1) {
    perror("fseek");
    return 0;
}
records = size / sizeof(x);
for (count = 0; count < records; count++) {
     if (fread(&x, sizeof(x), 1, file) == 1) {
         printf("\n\nID:%d",x.ID); /* %d instead of %ld */
     } else {
         break;
     }
}

But notice that you are always writing to the same variable on the stack.

EDIT:

How do you store the struct in the file?

I'm not storing it, a program is.

If it is not yours (you don't build the file using the same struct) then you can't know which is the sizeof(x) inside the file, read about structure padding and packing.

Community
  • 1
  • 1
David Ranieri
  • 36,077
  • 6
  • 44
  • 85
  • hmmn where is x created? – Ke. Sep 17 '16 at 08:38
  • 1
    hmmm where is your minimal, complete and verfiable example? thats a way to avoid using the almost always wrong `feof` – David Ranieri Sep 17 '16 at 08:40
  • Hmmn, I still get multiple prints with your version. Clearly something is wrong with the way we are accessing this data. There is only one record, and it is printing many. Could this be because it's reading bytes? The example is about as minimal as I could get it. The file I cannot upload because its binary – Ke. Sep 17 '16 at 08:49
  • hmmm, of course, `fread` reads bytes, all `f` functions reads bytes :) how are you writing to this file? using `fwrite`? – David Ranieri Sep 17 '16 at 08:50
  • Yes, but somehow it's not printing out the correct data, so something must be going wrong... ID is just one field of data in a struct, It's strange that its so hard to access it... – Ke. Sep 17 '16 at 08:52
  • Show how you store the `struct` in the file, this could be the problem. – David Ranieri Sep 17 '16 at 08:53
  • Hi Alter, I'm not storing it, a program is. I have a python code that does exactly the same thing as my C code I posted, using the construct library, and it pulls the ID out just fine. Just in C or C++ I cant seem to do the same. Do I need to SEEK for the specific ID part of the struct, in order to pull it out? – Ke. Sep 17 '16 at 08:58
  • 1
    And how do you know the `size` of the struct in the file if it is not yours? – David Ranieri Sep 17 '16 at 08:59
  • I dont seem to need the size in the python program Im using. Is it necessary to seek using offsetoff() function to the ID section of the struct, and then extract the data x bytes from there? – Ke. Sep 17 '16 at 09:17
  • @Ke, by chance your python program uses the good size, compare the size used by python and the `sizeof(x)`, they must be equal. – David Ranieri Sep 17 '16 at 09:27
  • I still don't understand what size has to do with it? Yes, I can find the size. However, I still cant seem to extract just the ID part of the struct. Noone has been able to explain that... – Ke. Sep 17 '16 at 10:08
  • I see what you are saying about size now. I know that there is only one struct in the whole file, so there's no need to divide by the number of records. I still can't access the ID part of the struct - it still prints out multiple numbers, which have nothing to do with the actual data. – Ke. Sep 17 '16 at 10:11
  • 1
    @user3121023, right! `ftell` result must be a multiple of `sizeof(x)` – David Ranieri Sep 17 '16 at 10:20
  • user3121023 - for the last time, there is only one struct record in my file, so the size of the struct is the same as the size of the whole file. The size is not my problem. Accessing that part of the struct is the problem. It should be as simple as x.ID but isnt... – Ke. Sep 17 '16 at 10:30
  • It is some middle element, I just gave the struct with ID at the top as an example. I can gather data from other parts of the struct, no problem and go over the bytes to get that section of the struct, but I cant seem to do it with this ID section, it prints out multiple times. I've got exactly the same file working fine with the same routine in python, just cant do it in c – Ke. Sep 17 '16 at 10:40
  • but surely I can find all these values using the offsetof function? This will give me the difference between that section and the other sections, therefore allowing me to read – Ke. Sep 17 '16 at 11:05
  • but it isnt wrong. This is a standard SDK file Im using, I'm sure everything is correct about it.I'm even sure its easily readible because Im doing that in python no problems, I can access all the data - including this ID section. I just cant seem to do that in C – Ke. Sep 17 '16 at 11:07
  • no, no elements are pointers. I believe the correct way to access a struct member is to use the offsetof function, to find the section of the member, and use that to fseek the document to the correct amount of bytes from the fseek location. have you got a better way? – Ke. Sep 17 '16 at 11:10
  • At least OP's code did `if(feof(p)!=0)` before using `x`. This code does `fread(&x, sizeof(x), 1, file); printf("\n\nID:%d",x.ID);` which is inferior as code does not know `x` contents is valid before using `x.ID`. Testing the turn value of `fread()` is the best solution. Sorry, but DV as this answer suggest " Why is “while ( !feof (file) )” always wrong?" but then made less resilient code. I am sure it is easily fixed. – chux - Reinstate Monica Sep 17 '16 at 15:02
  • @chux, notice that `records` takes care of the number of records in the file, `ftells` and a división is used before, but you are right, is better to check the result of `fread`, edited. – David Ranieri Sep 17 '16 at 15:20
  • True that the preceding code does reduce chance of problem. Corner cases still not covered (and recall OP is having trouble), `fread()` can always fail due to input errors (e.g. memory stick was removed), `records = size / sizeof(x);` is using mixed type math without range safe-guards (an issue with _huge_ files), and may others. IAC simplest and most direct to check `fread()` result. – chux - Reinstate Monica Sep 17 '16 at 15:23
1

Use more protection. Test the result of functions.

 FILE *p;
 struct myStruct x;
 p=fopen("myfile","rb");
 assert(p); // Insure file opened

 while(1) {
   size_t n = fread(&x, sizeof(x), 1, p);
   // feof() is insufficient, 
   // fread() can fail due to input errors too and not set end-of-file condition
   // if(feof(p)!=0)
   if (n == 0) {
     break;
   }
   // printf("\n\nID:%ld",x.ID);
   printf("\n\nID:%d", x.ID);  // Use matching specifier
   fflush(stdout); // Insure output occurs promptly
 }
 fclose(p);
 return 0;

Since OP's code had a mis-matched printf specifier, it indicates that either warnings are not fully enabled or OP is using a weak compiler. Suggest fixing that to save time.

chux - Reinstate Monica
  • 113,725
  • 11
  • 107
  • 213
  • Hi, it still prints out multiple lines, whereas I know that ID is only one value. In other words, it is not reading that ID part of the struct, but the whole struct. How do I make it just for the ID part? – Ke. Sep 17 '16 at 17:31
  • @Ke. " In other words, it is not reading that ID part of the struct," is contradicted by "but the whole struct.". Code cannot read the while `struct` without reading the filed `ID`. I suspect the true code and what you have posted here differ. 2 ideas: The `x` in `fread(&x, sizeof(x), 1, p);` is really a pointer or the file your are reading is much bigger than thought. What is the file size? – chux - Reinstate Monica Sep 17 '16 at 17:58