Inconsistent use of load_/save_override for serialized file signature

[Paul Barba <paul.barba@…>]

basic_binary_oarchive.ipp::39 writes the file signature like this:

• this->This() << file_signature;

Which if you're using your own binary_oarchive subclass to do custom serialization, ends up using your save_override(std::string&, int) function to write the signature.

However, basic_binary_iarchive.ipp::55 reads the file signature like so: std::size_t l; this->This()->load(l); if(l == std::strlen(BOOST_ARCHIVE_SIGNATURE())) { ... file_signature.resize(l); if(0 < l){ ... this->This()->load_binary(&(*file_signature.begin()), l); } }

which does not make use of your load_override(std::string&, int) function. Therefore if you change how strings are written, you get an invalid signature exception on deserialization.

[Paul Barba <paul.barba@…>]

Sorry about the formatting, basic_binary_iarchive.ipp::55 is

std::size_t l;
this->This()->load(l);
if(l == std::strlen(BOOST_ARCHIVE_SIGNATURE())) {
 ... 
 file_signature.resize(l); 
  if(0 < l){ 
    ... 
    this->This()->load_binary(&(*file_signature.begin()), l);
} } 

[Robert Ramey]

and your suggested patch is ...?

Robert Ramey

[Paul Barba <paul.barba@…>]

Well, you could just read the signature in basic_binary_iarchive.ipp like you write it:

this->This() >> file_signature; 

But I'm guessing the code was changed because that causes problems in some test case involving an invalid file? So rather than handling signatures with the user overloaded string functions, you could just do the saving of the signature string manually as well, something like the following in init() in basic_binary_oarchive.ipp:

std::size_t l = file_signature.size()
this->This()->save(l);
this->This()->save_binary(file_signature, l);

[Paul Barba <paul.barba@…>]

To add context, I went back and looked to see why a former colleague had overridden save_string() to begin with: Basically he switched the size_t for an unsigned int to induce x32/x64 compatibility into the binaries (the wisdom and correctness of such an operation being lengthy and off-topic.) In that sense supporting the load_override is preferable in that you retain the option of writing more portable string functions. However, since the goal of the binary archives is explicitly not 32/64 bit portability, it doesn't strike me as critical that that be supported here, only that the overrides be consistently used or ignored. Thus the first option is nicer in that judicious overrides can be used to overcome portability restraints, but I suspect there's a reason I don't know about that it's not done that way.

[Robert Ramey]

I checked in this change

this->This() >> file_signature;