C++ file handling read function error

Question

I am unable to find why this function isn't running correctly:

e1 is object of class employee:

class emp
{
    public: 
        int no;
        char name[30];

        void enter()
        {
            std::cout<<"\n enter no. and name";
            std::cin>>no;
            gets(name);
        }

        void disp()
        {
            std::cout<<"\n\tNo.\t\tName\n\t"<<no<<"\t\t"<<name;
        }
};

The program never goes in the if clause.

void modify(int a)
{
    f1.open("emp.txt", ios::ate | ios::nocreate);
    float pos;
    f1.seekg(0, ios::beg);
    while (!f1.eof())
    {
        f1.read((char*)&e1, sizeof(e1));
        if (e1.no == a) // if e1.no == the number to be modified
        {
            pos = f1.tellg();
            pos = pos - sizeof(e1)
            e1.enter(); // enter new e1 details, function declared in class
            f1.seekp(pos, ios::beg);
            f1.write((char*)&e1, sizeof(e1)); // overwrite prev rec.
            cout << "\n modified";
        }
    }
    f1.close();
}

Answer 1

You cannot de-serialize a class like so. Overwriting memory from instance base pointer will most likely wreak havoc and send you in Undefined Behavior land. Object memory layout needs to be considered if you wish to read like that(Hint: it'll most likely not work unless your class is really simple).

For example, if Employee class inherits from another class, the first member of the class is usually a pointer to the virtual table. If you read like this, you'll override the vtable pointer. Not good.

It is possible to read a C-struct like so but as soon as it contains pointers, it's pretty much over because next time you read, pointed to memory might not even be yours.

Since you are using C++ with possibly member like string, vector, etc., serialization like this is guaranteed to fail since they use dynamic memory allocation under the hood.

---

EDIT

Since you posted the definition of emp, I think I know what is happening. I assume that emp.txt is a file written by another process(or manually).

I've written a small piece of code to demonstrate why you would have problem reading your object like you do:

emp e;
std::cout << "sizeof(emp) = " << sizeof(emp) <<std::endl;
std::cout << "sizeof(emp::no) = " << sizeof(e.no) <<std::endl;
std::cout << "sizeof(emp::name) = " << sizeof(e.name) <<std::endl;

std::cout << "&e = " << &e <<std::endl;
std::cout << "&e.no = " << &e.no <<std::endl;
std::cout << "&e.name = " << &e.name <<std::endl;

and here is the output I got for a run:

sizeof(emp) = 36
sizeof(emp::no) = 4
sizeof(emp::name) = 30
&e = 00BBFDF4
&e.no = 00BBFDF4
&e.name = 00BBFDF8

We can see that no is at the beginning of the object: good. We can also see that name is located right after in memory( 00BBFDF8 - 00BBFDF4 == sizeof(emp.no) ). However, the size of the object is 36 when we would think it should be 34. This is because of alignment. The default one on my system is 4.

I doubt the data in emp.txt contains 2 extra bytes to cope with memory alignment. So by reading using sizeof(e1), you read 2 extra bytes. You are then in the middle of the next integer. This will obviously corrupt the data you are expecting on the next read.

Then there is also a problem with reading name. Even if alignment and memory layout where exactly like your program assumes, your text file would need to have been written according to sizeof(name) so every employee name would have to be padded with extra characters(space?) to be of fixed size(30 bytes). I doubt the input file is written like that.

I suggest you implement reading logic based on your input file instead of on the class you are using to store data. Also, editing the file in place will be hard if data is not of fixed size.

Answer 2

You are missing ios::binary in the flags argument of fstream::open(): for sure you are doing binary I/O here.

All the considerations of the other comments remain true however: you should have added (should add, if this not solving the issue) the definition of e1 at least.