2

I was reading Thinking in C++, and found this piece of code:

//: C06:Persist1.cpp
// Simple persistence with MI

#include <iostream>
#include <fstream>
using namespace std;
class Persistent {
    int objSize; // Size of stored object
    public:
    Persistent(int sz) : objSize(sz) {}
    void write(ostream& out) const {
    out.write((char*)this, objSize);
    }
    void read(istream& in) {
        in.read((char*)this, objSize);
    }
};
class Data {
    float f[3];
    public:
    Data(float f0 = 0.0, float f1 = 0.0,
        float f2 = 0.0) {
        f[0] = f0;
        f[1] = f1;
        f[2] = f2;
    }
    void print(const char* msg = "") const {
        if(*msg) cout << msg << " ";
        for(int i = 0; i < 3; i++)
        cout << "f[" << i << "] = "
        << f[i] << endl;
    }
};
class WData1 : public Persistent, public Data {
public:
    WData1(float f0 = 0.0, float f1 = 0.0,
        float f2 = 0.0) : Data(f0, f1, f2),
    Persistent(sizeof(WData1)) {
        cout<<"size of w1 "<<sizeof(WData1)<<endl;
    }
};
class WData2 : public Data, public Persistent {
public:
    WData2(float f0 = 0.0, float f1 = 0.0,
        float f2 = 0.0) : Data(f0, f1, f2),
    Persistent(sizeof(WData2)) {
        cout<<"size of w2 "<<sizeof(WData2)<<endl;
    }
};
int main() {
    {
        ofstream f1("f1.dat"), f2("f2.dat"),f3("f3.dat"), f4("f4.dat");
        WData1 d1(1.1, 2.2, 3.3);
        WData2 d2(4.4, 5.5, 6.6);
        WData1 d3(1.1, 2.2, 3.3);
        WData2 d4(4.4, 5.5, 6.6);
        d1.print("d1 before storage");
        d2.print("d2 before storage");
        d3.print("d3 before storage");
        d4.print("d4 before storage");
        d1.write(f1);
        d2.write(f2);
        d3.write(f3);
        d4.write(f4);
    } // Closes files
    ifstream f1("f1.dat"), f2("f2.dat"),f3("f3.dat"), f4("f4.dat");

    WData1 d1;
    WData2 d2;
    WData1 d3;
    WData2 d4;
    d1.read(f1);
    d2.read(f2);
    d3.read(f3);
    d4.read(f4);
    d1.print("d1 before storage");
    d2.print("d2 before storage");
    d3.print("d3 before storage");
    d4.print("d4 before storage");
} ///:~

It produces unexpected output: Objects of class WData1 are persisted correctly, but objects of WData2 aren't. While trying to find the source of problem, and possible fix, I found out, that problem is only in reading WData2 (its stored correctly in the file). To make this code work as intended, I had to change inheritance order from:

 WData2 : public Data, public Persistent{...

to

 WData2 :  public Persistent, public Data{...

I'm curious why inheritance order makes difference in this case. Shouldn't it make no difference?

mlecz
  • 985
  • 11
  • 19

3 Answers3

4

The problem is that in the class Persistent you use this to point to the beginning of the memory you want to read/write. This is a problem because you use this as an inherited class. Thus unless Persistent is not the first class inherited, this will not point to the beginning of the base class:

This is what we have for Wdata1 and Wdata2 (padding and alignment ignored):

      Wdata1
      +-----------+----------------------+
      | objSize   |  f[0]   f[1]   f[2]  |
      +-----------+----------------------+
      |Persistent |        Data          |
      ^
      |
this (of Persistent)

      |<-   what you read/ write       ->|



      WData2
      +----------------------+-----------+
      |  f[0]   f[1]   f[2]  | objSize   |
      +----------------------+-----------+
      |        Data          | Persistent|
                             ^
                             |
                    this (of Persistent)

                             |<-   what you read/ write       ->|
bolov
  • 72,283
  • 15
  • 145
  • 224
  • This kind of persistance looks awful, but it works in my machine. The problem is that, 2nd one doesn't. I read the strucure of the same class that I write, so data member layout shouldnt matter, as I read and write entire content of the class IMO – mlecz Jul 16 '14 at 07:15
  • Lets look at WData2. It's layout may look like: float float float int. There maybe optional padding. As machine saves the memory from begging of the object, and ending at the begging of the object+its size, it should restore that object after writing content to memory that previously was read. – mlecz Jul 16 '14 at 07:30
  • see my edit about `this`. That is your core problem. But the rest is still worth considering. – bolov Jul 16 '14 at 07:36
  • I still don't understand. "this" at WData2 points to Persistent part of class, which isn't beggining? Then if i read from this, ending at this+size i read the content of memory of Persistent and some that doesn't belong to WData2 object, but something else? – mlecz Jul 16 '14 at 07:45
  • 1
    yes, because you don't use `this` of `Wdata2`, you use `this` of `Persistent`. – bolov Jul 16 '14 at 07:49
  • Thanks for the credit, but my answer is useless, as I noted in an update. – rodrigo Jul 16 '14 at 08:18
2

The problem is in the calls in Persistent:

out.write((char*)this, objSize);
in.read((char*)this, objSize);

Here, this is a Persistent* and it is assumed to point to the beginning of the full object. That is only true if Persistent is the very first base class.

To work everywhere you can do this little known trick:

void *that = dynamic_cast<void*>(this);
out.write((char*)that, objSize);
in.read((char*)that, objSize);

The dynamic_cast<void*>() guarantees that you'll have the pointer to the most derived object.

WARNING!: Alas! This will not work unless the type is polymorphic. And the Persistent trick will not work with polymorphic types. So this answer, as it is, is useless. I am not deleting it because it may still be interesting why you shouldn't do this.

rodrigo
  • 94,151
  • 12
  • 143
  • 190
0

The cause of your problem might be cause your base-class initialization in the WData2 constructor initialization list. You need to call the base constructors in the same order you inherit the base classes.

So change

WData2(...) : Data(...), Persistent(...) { ... }

to

WData2(...) : Persistent(...), Data(...) { ... }
Some programmer dude
  • 400,186
  • 35
  • 402
  • 621