-2

I want to know why the code does not print the output. What is the difference between binary and normal file modes?

#include<stdio.h>
#include <stdlib.h>
typedef struct book_details
{ 
  char book_title[50];
  int book_no;
  float book_price;
}book_details;

int main()
{
  book_details b;
  FILE *fp;

  fp = fopen("book_list.txt","w+");
  if (fp == NULL)
      printf("File not found");

  fflush(stdin);
  printf("Enter Book Title: \n");
  gets(b.book_title);
  printf("Enter Book ID Number: \n");
  scanf("%d",&b.book_no);
  printf("Enter Book Price: \n");
  scanf("%f",&b.book_price);
  fprintf(fp,"Here are the book details");
  fwrite(&b,sizeof(b),1,fp);
  while (fread(&b,sizeof(b),1,fp) > 0)
      printf("%s %d %f\n",b.book_title,b.book_no,b.book_price);
  fclose(fp);
 }

What are the mistakes?

Jonathan Leffler
  • 730,956
  • 141
  • 904
  • 1,278
dsaharia
  • 67
  • 2
  • 13
  • Compile with all warnings & debug info: `gcc -Wall -Wextra -g` with [GCC](http://gcc.gnu.org/). Improve your code to get no warnings. Add `\n` at end of `printf` format strings (since `stdout` is buffered). [Use the `gdb` debugger](https://sourceware.org/gdb/current/onlinedocs/gdb/) to understand the behavior of your program. StackOverflow is not a fix-my-code service, so your question is **off-topic** – Basile Starynkevitch Jun 01 '18 at 05:24
  • 3
    As a beginner programmer, debugging is one of the most important skills for you to learn. Read https://ericlippert.com/2014/03/05/how-to-debug-small-programs/ for some tips on how to start. – Code-Apprentice Jun 01 '18 at 05:24
  • 3
    Note that [the `gets()` function is too dangerous to use — ever!](https://stackoverflow.com/questions/1694036/why-is-the-gets-function-dangerous-why-should-it-not-be-used) – Jonathan Leffler Jun 01 '18 at 05:31
  • 1
    You must have a positioning operation (e.g. `fseek()` when switching between reading and writing or vice versa. When you write one entry, you are at the EOF (the file was truncated when it was created), so the first read operation fails. – Jonathan Leffler Jun 01 '18 at 05:34
  • 1
    [**fflush(stdin); leads to undefined behaviour on most systems.**](https://stackoverflow.com/a/38325926/2173917). Don't use it. It is only valid on **seekable** streams. Which only applies to `stdin` if a file is *redirected* as input (e.g. `./prog – David C. Rankin Jun 01 '18 at 05:40
  • Possible duplicate of [Why is the gets function so dangerous that it should not be used?](https://stackoverflow.com/questions/1694036/why-is-the-gets-function-so-dangerous-that-it-should-not-be-used) – R.D Jun 01 '18 at 08:11

2 Answers2

0

This happens because here same File Pointer fp is used for reading and writing.Your output file is a binary file so only fread() and fwrite() can be used here. You cannot use fprintf(fp,"Here are the book details"); in this case.This also cause error in reading.In such cases there are TWO solutions.

  1. Using rewind().

Using the function rewind() we could rewind the File Pointer fp back to initial state to read the file.

Try this code :-

#include<stdio.h>
#include <stdlib.h>
typedef struct book_details
{ 
  char book_title[50];
  int book_no;
  float book_price;

}book_details;

int main()
{
  book_details b;
  FILE *fp;

  fp = fopen("book_list.txt","r+");
  if (fp == NULL)
     printf("File not found");

  printf("Enter Book Title: \n");
  gets(b.book_title);
  printf("Enter Book ID Number: \n");
  scanf("%d",&b.book_no);
  printf("Enter Book Price: \n");
  scanf("%f",&b.book_price);            // removed fprintf();
  fwrite(&b,sizeof(b),1,fp);
  fflush(stdin);

  rewind(fp);                        // Using rewind();

  while (fread(&b,sizeof(b),1,fp) > 0)
     printf("%s %d %f\n",b.book_title,b.book_no,b.book_price);
  fclose(fp);
 }
  1. Using separate read and write FILE pointers.

Try this code:-

#include<stdio.h>
#include <stdlib.h>
typedef struct book_details
{ 
  char book_title[50];
  int book_no;
  float book_price;

}book_details;

int main()
{
  book_details b;
  FILE *fpwrite,* fpread;                 // separate File pointers. 

  fpwrite = fopen("book_list.txt","w");
  if (fpwrite == NULL)
     printf("File not found");

  printf("Enter Book Title: \n");
  gets(b.book_title);
  printf("Enter Book ID Number: \n");
  scanf("%d",&b.book_no);
  printf("Enter Book Price: \n");
  scanf("%f",&b.book_price);                 // removed fprintf();
  fflush(stdin);
  fwrite(&b,sizeof(b),1,fpwrite);
  fclose(fpwrite);

  fpread = fopen("book_list.txt","r");
  while (fread(&b,sizeof(b),1,fpread) > 0)
     printf("%s %d %f\n",b.book_title,b.book_no,b.book_price);
  fclose(fpread);
 }

Separate File pointers are considered better since it improves Readability of source-code.

anoopknr
  • 3,177
  • 2
  • 23
  • 33
  • Recall, when you answer questions on StackOverflow, you step into the role of teacher. Using `gets()` is never acceptable. It is so prone to exploit by buffer overrun is has been completely removed from the C11 Standard. Don't use it, even if the questioner did. In answering you should be correcting errors and insecurities, not propagating them. You must also validate the `scanf` **return** every time -- otherwise you can have no confidence you are not processing garbage after a *matching* or *input* failure (or the user canceling by generating a manual `EOF`). – David C. Rankin Jun 01 '18 at 06:54
0

You have a large number of errors in your code. Most can be overcome by simply changing the way you write-to and then read-from your data file. There is no need to open the file in "w+" mode. You (1) write data, and then (2) read data. So simply open the file originally in append mode "a" (or "ab" to explicitly specify a binary write -- not required, but it does make what your are doing clear). Then close and open the file again to read.

When you write your book info out. Do NOT write "Here are the book details" to the file (it will break your read of struct from the file unless you offset the file position indicator beyond the unneeded text before starting your read of data).

Then close your file, you are done writing (validating the return of fclose after a write to catch any stream error not reported as you validated each write). Now simply open your file again in "r" (read) mode and loop over each struct read and outputting the values to the screen.

Rather than going over each error (including the use of gets() which from this day forward you will never never never do again) and the fflush (stdin) which is not supported by all OS's except for seekable streams, I have included comments inline below explaining how to handle the user-input, the write and the subsequent read. Aside from the workings of the code, the most important point I can make is to validate each and every input and output your code makes. That will save you untold amounts of time when things go wrong...

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

#define MAXT 50         /* if you need a constant #define one (or more) */
#define FNAM "book_list.txt"
#define DETAIL "Here are the book details"

typedef struct book_details { 
    char book_title[MAXT];
    int book_no;
    double book_price;  /* don't use floating-point variables for currency */
} book_details_t;       /* people get upset when you lose $ from rounding. */

int main (int argc, char **argv) {  /* use arguments to pass info to main  */

    char *filename = argc > 1 ? argv[1] : FNAM; /* read from file given as */
    book_details_t b = { .book_title = "" };    /* argv[1] or FNAM if none */
    size_t len = 0;             /* string length to aid with trimming '\n' */
    FILE *fp;

    fp = fopen (filename, "ab");    /* no need for "w+", "a" (append) will
                                     * let you write new values to file,
                                     * then reopen in "r" to read values
                                     */
    if (fp == NULL) {
        perror ("fopen-filename");  /* if the file isn't open, don't just  */
        return 1;                   /* output a msg, handle the error.     */
    }

    // fflush(stdin);       /* only valid on seekable streams or some OSs  */

    printf ("Enter Book Title: ");
    fgets (b.book_title, MAXT, stdin);      /* use fgets - NEVER ever gets */
    len = strlen (b.book_title);            /* get length of title */
    if (len && b.book_title[len-1] == '\n') /* check len & ends with '\n'  */
        b.book_title[--len] = 0;            /* replace '\n' with '\0' (0)  */
    else if (len == MAXT - 1) {             /* otherwise title too long    */
        fprintf (stderr, "error: title too long.\n");   /* handle error    */
        return 1;
    }

    printf ("Enter Book ID Number: ");
    if (scanf ("%d", &b.book_no) != 1) {    /* must validate scanf return  */
        fprintf (stderr, "error: invalid book_no.\n");      /* every time! */
        return 1;
    }

    printf("Enter Book Price: ");           /* same thing for every input  */
    if (scanf ("%lf", &b.book_price) != 1) {
        fprintf (stderr, "error: invalid book_price.\n");
        return 1;
    }

    printf ("\n%s\n\n", DETAIL);  /* don't write to file - will break read */

    if (fwrite (&b, sizeof(b), 1, fp) != 1) {   /* validate every write    */
        perror ("fwrite-b");
        return 1;
    }
    if (fclose (fp) == EOF) {           /* validate 'close after write'    */
        perror ("fclose after write");
        return 1;
    }

    if ((fp = fopen (filename, "r")) == NULL) { /* validate open for read  */
        perror ("open for read");
        return 1;
    }

    while (fread (&b, sizeof(b), 1, fp) > 0)
        printf ("%s %d %f\n", b.book_title, b.book_no, b.book_price);

    fclose(fp);

    return 0;
}

(note: while C99+ will automatically return 0; at the end of main(), it is a good idea to include it)

Example Use/Output

$ ./bin/struct_read_after_write dat/struct_books.dat
Enter Book Title: Huck Finn
Enter Book ID Number: 10157
Enter Book Price: 19.99

Here are the book details

Huck Finn 10157 19.990000

Add next book:

$ ./bin/struct_read_after_write dat/struct_books.dat
Enter Book Title: Tom Sawyer
Enter Book ID Number: 10156
Enter Book Price: 22.95

Here are the book details

Huck Finn 10157 19.990000
Tom Sawyer 10156 22.950000

Check Binary File

$ hexdump -Cv dat/struct_books.dat
00000000  48 75 63 6b 20 46 69 6e  6e 00 00 00 00 00 00 00  |Huck Finn.......|
00000010  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
00000020  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
00000030  00 00 00 00 ad 27 00 00  3d 0a d7 a3 70 fd 33 40  |.....'..=...p.3@|
00000040  54 6f 6d 20 53 61 77 79  65 72 00 00 00 00 00 00  |Tom Sawyer......|
00000050  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
00000060  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
00000070  00 00 00 00 ac 27 00 00  33 33 33 33 33 f3 36 40  |.....'..33333.6@|
00000080

All good, but notice the wasted space from writing MAXT (50) chars for every title? (if you had not initialized b = { .book_title = "" }; what do you think would be in the file?) Later on you will want to serialize the data and only write the bytes containing data to the file, and precede the title by the number of characters to read -- but that is for another day.

Tweak to printf Formatting

You may also want to tidy up your output formatting a bit by including the field width modifier and left justification for the title, providing a fixed-width for the ID, and limiting the precision of the price to 2-places, e.g.

    while (fread (&b, sizeof(b), 1, fp) > 0)
        printf ("%-50s %6d    $%.2f\n", 
                b.book_title, b.book_no, b.book_price);

Which would then produce tidier output of:

$ ./bin/struct_read_after_write dat/struct_books.dat
Enter Book Title: Validating All I/O
Enter Book ID Number: 101
Enter Book Price: 99.50

Here are the book details

Huck Finn                                           10157    $19.99
Tom Sawyer                                          10156    $22.95
My Dog with Fleas                                   10150    $9.99
Lucky Cats Have None                                10151    $12.49
Fun with C                                            100    $59.21
Validating All I/O                                    101    $99.50

Look things over and let me know if you have further questions.

David C. Rankin
  • 81,885
  • 6
  • 58
  • 85