[boost] Re: Serialization Library Review

From: Alberto Barbati (abarbati_at_[hidden])
Date: 2002-11-13 14:36:10

Robert Ramey wrote:
>>From: Alberto Barbati <abarbati_at_[hidden]>
>>1) I don't like the non-intrusive way of specifying the 
>>version/save/load operation.
> a non-intrusive method is required to implement serialization for classes that
> you don't want to change.  For example, the library includes serialization
> for all STL containers with without changing STL itself.  This would
> permit easy and optional addition of serialization to any class that
> might benefit from it

As pointed out by Vladimir, I am not disputing the presence of the 
non-intrusive method, which is indeed necessary. It's the way that is 
implemented (through template specialization) that I don't like. Free 
functions are a much better option, in my opinion.

>>2) A most needed addition to the design is to provide a sort of 
> "registry" object.....
> 
> This has been a hot topic.  It is really not possible to achieve the
> desired results.  I will add a section to th rationale explaining this
> in detail, 

Please note that the "registry" class I described *does not* attempt to 
solve the broader issue of UUIDs I read about in the discussion between 
you and Vladimir. My proposal is just a way to separate the 
"registration" part from the "serialization" into two different 
compontents. Still the classes will be matched according to their 
registration order, as it happens now. I am ready to discuss the 
opportunity and/or usefulness of this approach, but I don't see the 
reason why this could not be done.

>>One note: the library, as it is, *does not* support Unicode output, as 
>>stated. The library supports wide streams, yes, but that does not mean 
>>Unicode support.
> So what do I have to do exactly in the warchive specialization to generat
> Unicode output?

As I said in my post, you have to imbue the stream with a locale holding 
the correct codecvt facet, before using it to serialize. Unfortunately, 
such a facet is not part of the ANSI standard and is the subject of a 
separate proposal of mine (see thread "codecvt facets for 
utf8/utf16/utf32").

BTW, with the current implementation even doing so is completely 
useless, as there are lines like this:

     os.imbue(std::locale::classic());

that reset the locale of the streams to the "dumb" default. Such lines 
are IMO both unnecessary and conceptually wrong, and should be removed.

On a different topic, I found a portability issue. The current 
implementation record in the archives the size of the basic types int, 
long, float and double. This gives you a *false* sense of security that 
the "writing" and "reading" platform agree on the type size. First 
objection: you should check also number of bits of a char, which is not 
necessarily 8 bits, and the size of shorts. However, it gets worse. On 
both x86 and i64 plaforms int, long and float are 4 bytes, while double 
is 8 bytes, so they "agree" according to this test. However, type size_t 
on x86 has 4 bytes, while on i64 has 8 bytes. It's not hard to imagine 
an application that tries to serialize a size_t. The library won't 
detect the issue, but the compiler will use a different overloads of 
operators << and >> with clearly bad consequences. The same problem may 
happen with other typedef'd types. Maybe it would be a good idea to add 
in the documentation a note warning about this case and encouraging the 
use of fixed-size types (like boost::int8_t, etc.) for the members of a 
serializable class, at least when multi-plaform is an issue.

Moreover, this issue is now present in library itself! In file 
archive.cpp, function write_string() writes the length of the string as 
a size_t, while read_string() reads it as an unsigned int, which may 
have a different size.

Alberto Barbati