Why does the program only save some, but not all, of data to binary file?

Question

After write data to the binary file, I changed the mode to "r" to read the file. The name of who is correct, but color and education are empty. Age is returned as a large integer number, which is I guess the address of the variable. So, what is wrong here?

Update: The answer of Retired Ninja and Thornkey almost solve my problem. The rest is if input of age is 26, but not other numbers, the program will not write correct input to file. Anyone know what is wrong here?

#include <stdio.h>
#include <string.h>
#include <stdlib.h>
#define MAXLEN 100


typedef struct Person{
    char name[MAXLEN];
    int age;
    char color[MAXLEN];
    char education[MAXLEN];

} Person;

void create_person(Person *who){
    printf("name: ");
    fscanf(stdin, "%s", who->name);
    printf("age: ");
    fscanf(stdin, "%d", &(who->age));
    printf("color: ");
    fscanf(stdin, "%s", who->color);
    printf("education: ");
    fscanf(stdin, "%s", who->education);


}

void print_record(Person *who){
    printf("name: %s\n", who->name);
    printf("age: %d\n", who->age);
    printf("color: %s\n", who->color);
    printf("education: %s\n", who->education);

}

void load_db(FILE *fp, Person *who){
    int result = fread(who, sizeof(who), 1, fp);
    if(!result)
        //printf("result%d", result);
        printf("cannot load database");
}


FILE *connect_db (char *file_name, char *mode, Person *who){
    FILE *fp = (FILE *)malloc(sizeof(100));
    // open stream and load database from the file
    if(strcmp(mode, "w") == 0){
        fp = fopen(file_name, mode);
        //load_db(conn); // load data from file
    }else if(strcmp(mode, "r") == 0){
        fp = fopen (file_name, mode);
        load_db(fp, who); // load data from file
    }else{
        printf("incorrect mode");

    }

    return fp;

}

// save database to file
int save_db (FILE *fp, Person *who){

        int result = fwrite(who, sizeof(who), 1, fp);

        if(result){

            return 0; // successfully save db

        }
        printf("cannot save db");




}

int main(int argc, char *argv[])
{
    char answer[MAXLEN];


    Person person;
    Person *who = &person;
    FILE *fp;
    create_person(who);

    fp = connect_db("record2.dat", "w", who);

    save_db(fp, who);
    print_record(who);
    free(fp);
    fclose(fp);


    return 0;
}

Answer 1

Here's a fixed version that may be of some help to you. It successfully fills in a Person, prints it, and writes it to the file. Then it reads the data back from the file into a different Person and prints it.

The structure is similar to what you had, but I made all the reading/writing explicit so you'd see the steps. In general it isn't a good idea to have functions that perform extra duties you might not always want. I also made create_person require no input for faster testing. Your input code looked okay, I just didn't want to type it every time.

One thing to keep in mind, if you plan to write binary data to a file you should open the file in binary mode ("wb" or "rb") to avoid line ending translation on systems that perform that on text files.

You might also consider that a file written on one system may not be readable on a different system if the size of the Person structure changes due to different alignment or int being a different size. Probably not an issue for you, but if it becomes one you might look into a different serialization scheme.

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

#define MAXLEN 100
typedef struct Person
{
    char name[MAXLEN];
    int age;
    char color[MAXLEN];
    char education[MAXLEN];

} Person;

void create_person(Person *who)
{
    strcpy(who->name, "Fred Smith");
    who->age = 21;
    strcpy(who->color, "Red");
    strcpy(who->education, "Some School");
}

void print_record(Person *who)
{
    printf("name: %s\n", who->name);
    printf("age: %d\n", who->age);
    printf("color: %s\n", who->color);
    printf("education: %s\n", who->education);
}

void load_db(FILE *fp, Person *who)
{
    int result = fread(who, sizeof(*who), 1, fp);
    if(!result)
        printf("cannot load database");
}

FILE *connect_db(char *file_name, char *mode, Person *who)
{
    FILE *fp = NULL;
    if(strcmp(mode, "w") == 0)
    {
        fp = fopen(file_name, mode);
    }
    else if(strcmp(mode, "r") == 0)
    {
        fp = fopen(file_name, mode);
    }
    else
    {
        printf("incorrect mode");
    }
    return fp;
}

int save_db(FILE *fp, Person *who)
{
    int result = fwrite(who, sizeof(*who), 1, fp);
    if(result)
    {
        return 0;
    }
    printf("cannot save db");
    return -1;
}

int main(int argc, char *argv[])
{
    FILE* fp = NULL;
    Person who1;
    Person who2;
    create_person(&who1);
    print_record(&who1);

    fp = connect_db("record2.dat", "w", &who1);
    save_db(fp, &who1);
    fclose(fp);

    fp = connect_db("record2.dat", "r", &who2);
    load_db(fp, &who2);
    print_record(&who1);
    fclose(fp);

    return 0;
}

Answer 2

fread() reads individual bytes. You want to read in numbers which you have printf'd.

Your files will look like this:

name: Samuel Thornkey
age: 24
colour: blue
education: PHD in computer science

But when you use fread, the program directly reads bytes from the file and fills them into the record. Your person will then contain:

char name[MAXLEN]: first MAXLEN characters i.e. "name: Samuel Thornkey\n  age: 24\ncolour: blue\n" or something similar
int age: the rest of the characters, encoded as bytes, hence very large number
char color[MAXLEN]: empty
char education[MAXLEN]: empty

Instead, use fscanf:

fscanf(fp,"name:%s ",&who->name);
fscanf(fp,"age:%d ",&who->age);

and so on.

Answer 3

It's likely to be this:

free(fp);
fclose(fp);

You are not permitted to free memory that wasn't given to you by malloc (or realloc).

And, yes, you may think you've allocated it inside connect_db but (1) that's totally unnecessary, and (2) you overwrite the pointer when you call fopen.

In addition, save_db is using the size of the who pointer which will most likely not be the same as the type it points to.

So, make the following changes:

• get rid of the call to malloc, just use FILE *fp; within connect_db.
• get rid of the free(fp) within main.
• in save_db, use sizeof(Person) rather than sizeof(who).