Fails to read data from binary file into array of structure pointers

Question

the following code writes/reads binary data to/from binary file. The writing is successful, even the reading is successful as per fread return value. But the last two or three values will always be junk, I have verified this by increasing decreasing array size.

If we read file right after writing then values would be all fine. But if we close the program once after writing and then run program again with first action as read then it would print last few values as junk. Why so ?

#include<stdio.h>
#include<stdlib.h>
#define MAX 7

void write(FILE *);
void read(FILE *);

int flag=0;

struct emp
{
char name[20];
int id;
float salary;
}*e[MAX];

void write(FILE *f)
{
int i=0,check=0;

flag=1;
for(i=0;i<MAX;i++)
{
    e[i]=malloc(sizeof(struct emp));
    if(e[i])
    {
        printf("\nEnter Name id salary\n");
        scanf("%s %d %f",e[i]->name,&e[i]->id,&e[i]->salary);
        //printf("\n----------%s %d %f\n",e[i]->name,e[i]->id,e[i]->salary);
        //fflush(stdin);
    }
    else
    {
        printf("\nError allocating Memory\n");
        exit(0);
    }
}
check= fwrite(*e,sizeof(struct emp),MAX,f);
if(check!=MAX)
{
    printf("\nerror writing to file\n");
}
else
  {
    printf("\nwritten successfully to file\n");
  }
}

void read(FILE *f)
{
int i=0;

if(flag==0) //reading file right after running program
{
    for(i=0;i<MAX;i++)
    {
        e[i]=malloc(sizeof(struct emp));
        if(e[i]==NULL)
        {
            printf("\nError Allocating Memory for read\n");
            exit(0);
        }
    }
}

if(fread(*e,sizeof(struct emp),MAX,f)==MAX)
{
    for(i=0;i<MAX;i++)
    {
        printf("\n%s %d %f\n",e[i]->name,e[i]->id,e[i]->salary);
    }
}
else
{
    printf("\nEither reading error or partial content read\n");
}
}

int main()
{
FILE *fp=NULL;
char a='a';

do
{
    printf("\nEnter w to write, r to read and e to exit\n");
    //scanf("%c",&a);
    a=getche();

    switch(a)
    {
        case 'w':
                fp=fopen("binary_ptr.exe","wb");
                if(fp==NULL)
                {
                    printf("\nError opening file to write\n");
                    exit(0);
                }
                write(fp);
                fclose(fp);
                break;
        case 'r':
                fp=fopen("binary_ptr.exe","rb");
                if(fp==NULL)
                {
                    printf("\nError opening file to read\n");
                    exit(0);
                }
                read(fp);
                fclose(fp);
                break;
        case 'e':
                exit(0);
                break;
        default:
                printf("\nInvalid input\n");
                break;
    }
}
while(1);

return 0;
}

Answer 1

On reading/writing the code as shown assumes all structs are placed in a continuous region of memory starting with *e. This is not the case.

The code does not define an array of struct but of pointers to the latter. The dynamically allocated structs themselves are scattered all over memory.

As it stands the code invokes undefined behaviour for all MAX > 1 by writing/reading beyond *e[0]'s bounds.

To fix this loop around fread()/fwrite() MAX times for one struct each.

^{(To be pointed to this issue explicitly run the compiled code using the Valgrind memory checker, for example.)}

---

As side note unrelated to your issue: read() and write() are bad names for one owns functions as the are already used by the POSIX C Standard.

Answer 2

You can't do (fread(*e, sizeof(struct emp), MAX, f) since you've allocated memory for your emp structs non-contiguously, as correctly noted, you have an array of pointers to emp structs, not an array of emp structs. You need to read each emp separately:

for (int i = 0; i < MAX; ++i){
    if (fread(e[i], sizeof(struct emp), 1, f) == sizeof(struct emp)){
        printf("\n%s %d %f\n", e[i]->name, e[i]->id, e[i]->salary);
    } else {
        ...
    }
}

Similarly, when you write emp data to file, you need to do it seperately too

for (int i = 0; i < MAX; ++i){
    check = fwrite(e[i], sizeof(struct emp), 1, f);
    if (check != sizeof(struct emp)){
        ...
    } else {
    ...
    }
}

Answer 3

In the read function, you have

if(fread(*e,sizeof(struct emp),MAX,f)==MAX)

This reads MAX elements into an array e. However, e is not a contiguous array. You are allocating e separately with

e[i]=malloc(sizeof(struct emp));