Binary to textual error Correction

Question

I've been working on a program that reads from a binary file objects of students and writes them inside a textual file, but the binary file used is also obtained by reading the data of students from a textual file. I did two functions to do so, the textual to binary worked fine, still the binary to textual is giving me errors and I don't know why.

that's my code:

#include<stdio.h>
#include<stdlib.h>
#include<string.h>
 typedef struct a {
 char name[15];
 char surname[15];
int age;
}sID; 

int Binarycopy(char *s1,char *s2)     
{
sID id;
  FILE *f;
  FILE *g;
  char names[15];
  char surnames[15];
  int age;

  f=fopen(s1,"r");
  g=fopen(s2,"wb");// text file giving data about students, gathering them      into a data type structure and then write them in a binary file.


  if(f==NULL)
  { printf("File is empty");
      return -1;
  }
  fscanf(f,"%s %s %d",names,surnames,&age);

  while(!feof(f)){

      strcpy(names,id.name);
      strcpy(surnames,id.surname);
      id.age=age;                //write data into the ID object

      fwrite(&id,sizeof(sID),1,g); //write the ID object inside g

      fscanf(f,"%s %s %d",names,surnames,&age);
  }

  fclose(g);
  fclose(f);

  return 1;
}


 int Textcopy(char *s1,char *s2) 
{
sID id;
FILE *f;
FILE *g;

f=fopen(s1,"rb");
g=fopen(s2,"w");

if(fread(&id,sizeof(sID),1,f)!=sizeof(sID))
{ printf("file is empty");
    return -1;
}
else
    while(1)
    { fprintf(g,"%s %s %d",id.name,id.surname,id.age);

    if(fread(&id,sizeof(sID),1,f)!=sizeof(sID))
    { 
        fclose(f);
            fclose(g);
        return 1;
    }}}

  int main(int argc, char *argv[])
 {
   char b[]={"-b"};
   char t[]={"-t"};
   if(argc<4)
   { printf("Not enough arguments");
       return -1;}

   if(strcmp(argv[1],b)==0)
    Binarycopy(argv[2],argv[3]);

   if(strcmp(argv[1],t)==0)
    Textcopy(argv[2],argv[3]);

   return 1;
  }

I'm getting "file is empty" when trying to write from binary to textual file, still i don't know why even though my binary file is not empty

You are not validating your reads or writes. `while (fscanf(f,"%s %s %d",names,surnames,&age) == 3) {...}` would be much better than `while(!feof(f)){...}` [**Why is while ( !feof (file) ) always wrong?**](http://stackoverflow.com/questions/5431941/why-is-while-feof-file-always-wrong). You should check `if (fclose(g) == -1) {/* handle stream error after write */}` — David C. Rankin, Nov 28 '17 at 17:55
Why do you declare `char surnames[20];` in `Binarycopy` when you have `char surname[15];` in `struct a`? — David C. Rankin, Nov 28 '17 at 18:03

David C. Rankin · Accepted Answer · 2017-11-28T20:16:13.377

You have a number of problems with validation that prevent detecting when there are errors with file open, file read, file write and file close. Before getting there, if you need constants, #define them or use an enum to define them, e.g.

enum { NAMSZ = 15, MAXC = 512 };

(that way you don't have magic numbers sprinkled throughout your code. note: scanf field width modifiers are an exception -- they must be specified and cannot use variable or constant labels)

There is no need to declare typedef struct a {..., you don't make use of a, just use the typedef of the struct:

typedef struct {
    char name[NAMSZ];
    char surname[NAMSZ];
    int age;
} sid;

I've given you the link to Why is while ( !feof (file) ) always wrong?. Instead, declare a buffer sufficient to hold each line and read each line with fgets and then parse the values you need from the line using sscanf (or just a pointer and loop), e.g.

int binarycopy (char *s1, char *s2)
{
    sid id = { .name = "" };
    FILE *f, *g;
    char buf[MAXC] = "";

    f = fopen (s1, "r");
    g = fopen (s2, "wb");

    if (f == NULL || g == NULL) {
        fprintf (stderr, "binarycopy: file open failed.\n");
        exit (EXIT_FAILURE);
    }

    while (fgets (buf, MAXC, f)) {
        if (sscanf (buf, "%14s %14s %d", id.name, id.surname, &id.age) == 3) {
            if (fwrite (&id, sizeof id, 1, g) != 1) {
                fprintf (stderr, "error: fwrite error.\n");
                exit (EXIT_FAILURE);
            }
        }
        else {
            fprintf (stderr, "binarycopy error: invalid line format.\n");
            exit (EXIT_FAILURE);
        }
    }

    fclose (f);
    if (fclose (g) == -1) {
        fprintf (stderr, "error: on stream close.\n");
        exit (EXIT_FAILURE);
    }

    return 1;
}

(note: the validation of fclose after write. stream errors can occur that are not otherwise reported. Always validate fclose after write)

While you should serialize the data written to the binary file above (e.g. check the strlen of each name, write the length, then that number of chars, and then the age, for learning purposes, you can write a struct-at-a-time, but note: the data file is not guaranteed to work on another architecture or compiler due to differences in padding. You can write the struct and read it back in on the same compiler, but know serializing the data is the proper way to go.

For your read, simply do the reverse. fread a struct worth of data, and write the line to the text file, e.g.

int textcopy (char *s1, char *s2)
{
    sid id;
    FILE *f;
    FILE *g;

    f = fopen (s1, "rb");
    g = fopen (s2, "w");

    if (f == NULL || g == NULL) {
        fprintf (stderr, "textcopy: file open failed.\n");
        exit (EXIT_FAILURE);
    }

    while (fread (&id, sizeof id, 1, f) == 1)
        fprintf (g, "%s %s %d\n", id.name, id.surname, id.age);

    fclose (f);
    if (fclose (g) == -1) {
        fprintf (stderr, "error: on stream close.\n");
        exit (EXIT_FAILURE);
    }

    return 1;
}

In main() validate your options. If you don't get -b or -t handle the error. Also, there is no need to use strcmp, just check if the second char in argv[1] (e.g. argv[1][1]) is 'b' or 't', e.g.

int main (int argc, char *argv[])
{
    if (argc < 4) {
        printf ("Not enough arguments");
        return 1;
    }

    if (argv[1][1] == 'b')
        binarycopy (argv[2], argv[3]);
    else if (argv[1][1] == 't')
        textcopy (argv[2], argv[3]);
    else
        fprintf (stderr, "error: unrecognized option.\n");

    return 0;
}

Putting it altogether, you can do something like:

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

enum { NAMSZ = 15, MAXC = 512 };

typedef struct {
    char name[NAMSZ];
    char surname[NAMSZ];
    int age;
} sid;

int binarycopy (char *s1, char *s2)
{
    sid id = { .name = "" };
    FILE *f, *g;
    char buf[MAXC] = "";

    f = fopen (s1, "r");
    g = fopen (s2, "wb");

    if (f == NULL || g == NULL) {
        fprintf (stderr, "binarycopy: file open failed.\n");
        exit (EXIT_FAILURE);
    }

    while (fgets (buf, MAXC, f)) {
        if (sscanf (buf, "%14s %14s %d", id.name, id.surname, &id.age) == 3) {
            if (fwrite (&id, sizeof id, 1, g) != 1) {
                fprintf (stderr, "error: fwrite error.\n");
                exit (EXIT_FAILURE);
            }
        }
        else {
            fprintf (stderr, "binarycopy error: invalid line format.\n");
            exit (EXIT_FAILURE);
        }
    }

    fclose (f);
    if (fclose (g) == -1) {
        fprintf (stderr, "error: on stream close.\n");
        exit (EXIT_FAILURE);
    }

    return 1;
}


int textcopy (char *s1, char *s2)
{
    sid id;
    FILE *f;
    FILE *g;

    f = fopen (s1, "rb");
    g = fopen (s2, "w");

    if (f == NULL || g == NULL) {
        fprintf (stderr, "textcopy: file open failed.\n");
        exit (EXIT_FAILURE);
    }

    while (fread (&id, sizeof id, 1, f) == 1)
        fprintf (g, "%s %s %d\n", id.name, id.surname, id.age);

    fclose (f);
    if (fclose (g) == -1) {
        fprintf (stderr, "error: on stream close.\n");
        exit (EXIT_FAILURE);
    }

    return 1;
}

int main (int argc, char *argv[])
{
    if (argc < 4) {
        printf ("Not enough arguments");
        return 1;
    }

    if (argv[1][1] == 'b')
        binarycopy (argv[2], argv[3]);
    else if (argv[1][1] == 't')
        textcopy (argv[2], argv[3]);
    else
        fprintf (stderr, "error: unrecognized option.\n");

    return 0;
}

Example Input File

$ cat dat/nameage.txt
John Smith 30
Mary Jane 35
Dan Kane 55
Annie Adams 40

Example Use/Output

Copy to binary:

$ ./bin/filecopytb -b dat/nameage.txt dat/nameagecpy.bin

Copy to text:

$ ./bin/filecopytb -t dat/nameagecpy.bin dat/nameagecpy.txt

Compare:

$ diff dat/nameage.txt dat/nameagecpy.txt

Hexdump of binary:

$ hexdump -Cv dat/nameagecpy.bin
00000000  4a 6f 68 6e 00 00 00 00  00 00 00 00 00 00 00 53  |John...........S|
00000010  6d 69 74 68 00 00 00 00  00 00 00 00 00 00 00 00  |mith............|
00000020  1e 00 00 00 4d 61 72 79  00 00 00 00 00 00 00 00  |....Mary........|
00000030  00 00 00 4a 61 6e 65 00  00 00 00 00 00 00 00 00  |...Jane.........|
00000040  00 00 00 00 23 00 00 00  44 61 6e 00 00 00 00 00  |....#...Dan.....|
00000050  00 00 00 00 00 00 00 4b  61 6e 65 00 00 00 00 00  |.......Kane.....|
00000060  00 00 00 00 00 00 00 00  37 00 00 00 41 6e 6e 69  |........7...Anni|
00000070  65 00 00 00 00 00 00 00  00 00 00 41 64 61 6d 73  |e..........Adams|
00000080  00 00 00 00 00 00 00 00  00 00 00 00 28 00 00 00  |............(...|

cat of text:

$ cat dat/nameagecpy.txt
John Smith 30
Mary Jane 35
Dan Kane 55
Annie Adams 40

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

Glad to help. Just note for old C89 compilers (e.g. Windows 7), if you want to initialize `id` you can use `sid id = { {0},{0},0 };` as VS10 doesn't allow named initializers. You don't have to initialize `id` at all (you may just see stray garbage in the hexdump for bytes after the *nul-terminating* character for each string in the binary file that do not affect anything). Initializing the struct sets all bytes to zero (making for a better looking hexdump) Good luck with your coding. — David C. Rankin, Nov 29 '17 at 00:22

Binary to textual error Correction

1 Answers1