-1

I created a function that is successfully reading the binary file but it is not printing as I wanted.

The function:

void print_register() {

    FILE *fp;
    fp = fopen("data.bin", "rb");

    if (fp == NULL) {
        error_message("Fail to open data.bin for reading");
        exit(0);
    }

    reg buffer;

    while (EOF != feof(fp)) {
        fread(&buffer, sizeof(reg), 1, fp);
        printf("%s %d %d\n", buffer.name, buffer.age, buffer.id);
    }
    fclose(fp);
}

Note: reg is a typedef for a struct:

typedef struct registers reg;
struct registers {
    char name[30];
    int age;
    int id;
    char end;
};

Function for writing the file:

void register_new() {

    system("clear");

    reg buffer;

    FILE *fp;
    fp = fopen("data.bin", "ab");

    if (fp == NULL) {
        error_message("Error opening file data.bin");
        exit(0);
    }

    write_register(buffer);

    fwrite(&buffer, sizeof(reg), 1, fp);

    fclose(fp);
}

Posting a printscreen of what was print to be more helpful:

Console Image

As you can see on image, after the "p" (command for printing) is where should be the name, age and id of the struct.

chqrlie
  • 131,814
  • 10
  • 121
  • 189
  • I think that you need to post your code for the Save command too, seems like a problem there. – Mike Aug 20 '16 at 21:11
  • 1
    Where is the declaration of `reg`? Also, `while (!feof(fp)){` is [wrong](http://stackoverflow.com/questions/5431941/why-is-while-feof-file-always-wrong). – n. m. could be an AI Aug 20 '16 at 21:11
  • What is `reg`? It better have enough storage space for your string. What is in that file? It does not appear to contain text. Perhaps - if you created it as well - you stored a *pointer* to your text, instead of the actual text. – Jongware Aug 20 '16 at 21:12
  • Maybe it's not the printing part that's broken but `write_register()`. Did you make sure `data.bin` contains valid data? (you could use `hexdump` or similar tools to check your file's content). – yoones Aug 20 '16 at 21:21
  • @Mike did some edits, maybe it helps –  Aug 20 '16 at 21:22
  • You are open a file a binary: `fp = fopen("data.bin", "rb");`. Consider open it as text file: `fp = fopen("data.bin", "rt");`. You are using `printf("%s ... , `buffer.name`. You must make sure the C string buffer.name is NULL terminated (ends with zero). – Rotem Aug 20 '16 at 21:22
  • @RadLexus did put a note to say that, but did edit with the code for it –  Aug 20 '16 at 21:22
  • @Rotem the "rt" mode didn't work, also used scanf for inserting names so everything is null terminated. –  Aug 20 '16 at 21:24
  • You can't just use "rt", you need to modify all reading and writing operations to text operations. You can work with binary as well... – Rotem Aug 20 '16 at 21:30
  • "successfully reading the binary file" and `while (EOF != feof(fp)) { fread(&buffer, sizeof(reg), 1, fp); printf("%s %d %d\n", buffer.name, buffer.age, buffer.id); }` do not agree. See [this answer](http://stackoverflow.com/a/39058843/2410359). – chux - Reinstate Monica Sep 14 '16 at 22:05

2 Answers2

3

Your reading loop is incorrect. Don't use feof(), it can only tell is you have reached the end of file after a read attempt failed and it might not return EOF anyway, it is only specified as returning 0 or non 0. Use this instead:

while (fread(&buffer, sizeof(reg), 1, fp) == 1) {
    printf("%s %d %d\n", buffer.name, buffer.age, buffer.id);
}

fread returns the number of items successfully read. Here you request to read 1 item of size sizeof(reg), if the item was read successfully, fread will return 1, otherwise it will return 0 (in case of a read error or end of file reached).

Your screenshot shows a syntax error, which you seem to have fixed now. Remove that, it is not helping.

In your function register_new, you are writing an uninitialized structure reg to the file, no wonder it does not contain anything useful when you read it back from the file. And for what it is worth, opening this file in binary mode is the correct thing to do since it contains binary data, namely the int members of the structure.

The reg passed to fwrite is indeed uninitialized. write_register gets a copy of this uninitialized structure by value, and probably modifies this copy, but this does not affect the local structure in register_new. You should modify write_register() to take a pointer to the structure. Unlike C++, there is no passing by reference in C.

chqrlie
  • 131,814
  • 10
  • 121
  • 189
  • I get the why of the change, but why `== 1`? Does it return 0 when there is nothing more to read? –  Aug 20 '16 at 21:29
  • It is initialized, it is what the `write_register ( )` does. –  Aug 20 '16 at 21:35
3

In register_new(), you have to send the address of buffer in order for write_register() to work properly (right now you're giving it a copy of buffer).

Replace: write_register(buffer); with: write_register(&buffer);

Then correct write_register to take and work with an address instead of a structure.

This might help you understand what's going on: http://fresh2refresh.com/c-programming/c-passing-struct-to-function

yoones
  • 2,394
  • 1
  • 16
  • 20