- Boost - lists.boost.org

Re:[boost] Re:Formal review: serialization
by Robert Ramey 22 Apr '04

22 Apr '04

Vladimir Prus wrote: > Robert Ramey wrote: > > >> -- (critical) The primary issue with the library is lack of reference > >> documentation > ... OK I don't see a problem with this - Thanks to Dave for the hint on how to handle this. > For those stubs, requirements docs that Dave mentioned would be nice. > However, there's code that user calls, and I'd like to see docs for that, > too. > > For example: > > binary_object make_binary_object(void* ptr, unsigned size); > Returns: binary_object(ptr, size); > > binary_object::save(Archive& ar, unsigned version) > Effects: Calls ar.save_binary(m_ptr, m_size); > > binary_object::load(Archive& ar, unsigned version) > Effects: Calls ar.load_binary(m_ptr, m_size); > Aside - I wouldn't expect load/save/load_binary/save_binary to ever be invoked by a library user. > It would be nice to avoid asking the user to do BOOST_CLASS_EXPORT for all > possible argument types. What's desirable is: > > template<class T> > void register_rpc_function(const char* name, void (*f)(const T&)) > { > functions[name] = ... ; > boost::serialization::export_class< function_call_1<T> > >::instantiate(); > } > > I can't put BOOST_CLASS_EXPORT in register_rpc function now. This situation is discussed in the documentation under the heading "Template Serialization Traits". There is an example that uses assignation of traits to the nvp<T> template. BOOST_CLASS_EXPORT is just a syntactic short hand to for the specialization above. Uh-oh - I just looked at the definition of BOOST_CLASS_EXPORT and I see it's not that obvious as it is in the other traits. I'll take a look at this. > I'm afraid the no matter how smart XML reader is, it still would have to > scan the entire file. > So what's wrong with that? I would guess that such an XML reader already exists somewhere and you cat get everything thing for free. Assuming this doesn't address your need, and you want to do some programming, one can create another XML archive version. This would create two output files. One exactly as it is now will another "index". When I was considering the options regarding XML output I briefly considered the possibility of creating two files - one like we have the one now - along with an optional parallel file containing the corresponding XML schema describing the XML archive. I decided to keep things simpler. But I think you see the idea. > > I can expand the doc a bit. binary_object is just a wrapper around a > size > > and pointer to permit them to be handled as pair. It presumes the > pointer > > already points to allocated storage. > > This last sentence is exactly what I'd like added to docs. > OK that's easy. Sorry for the confusion. > > It also presumes that the the size > > of thing its pointing to is the same on save and load. Its very > > lightweight. > > > > I don't see a need for the others, but of course any user can make the > > wrappers he needs. > > The reason why I think the other is important, it that's it's actually > saving/loading support for plain C++ array -- which is rather basic thing. > Hmmm - the library already implements serialization of plane C++ arrays by serializing each element. This is the more general solution as it calls serialization for each element. The only time one might want to do save/load for a whole array is for a non-portable binary file. Re: save/load asymmetry for polymorphic pointers. > As I've said previously, this is a bug which is easy to make and very hard > to debug. I think we can only wait for others to express an opinion, as we > fail to convince each other. Well, THAT we can agree on. > > BTW, one usage of XML archives did occur to me. By checking the name > tag > > on input, we can implement a crude check that save and load operations > are > > synchronized. This would effectively a debug mode for archives and > might > > be useful. > > But you won't catch a case where you save one type and load another. True, but I think it would help a lot - and practically free to implement. > So, the get zero overhead I need to tweak base class an disable tracking > of > pointers. Let me try that... yea, the results are nice. One one extra > element (class id) per saved item. If your not serializing pointers, the class_id isn't written to the archive even once. Object id is required for tracking, class id is required for pointers. If versioning is used, class-id is used once. I do not believe that there is any information stored in an archive which is not used. > > BTW, how to I set tracking level and implementation level for a template > class. I think I can partially specialize 'tracking_level', but it should > be mentioned in the docs. See docs section "Template Serialization Traits" > > >> - The documentation should really state the minimal set of > 'load'/'save' > >> overloads which will make archive usable. For example, it's probably > not > >> necessary to provide separate overload for 'bool', right? > > > > The document states: > > > > " However, we're not quite done. The above code addresses serialization > of > > all non-primitive types. To be complete, each primitive type must either > > be covered by a definition of template<typename T> void load(T & t); or > an > > overload of the >> operator" > > It also necessary to provide overload for char*. Does not count as > primitive > type? Default implementation for char * will work as it does for other pointers. Which is probably not what one has in mind. I have code in there for handling it as a c string (Its commented out). I tested it and it worked but I came to conclude it presented a big security risk. The problem is the following: char * str = "abc" ar << str; // no problem the (text) archive looks like 3 abc later: char str[MAX_STRING_SIZE] ar >> static_cast<char *>(str); // to avoid str being treated as an array suppose the text archive gets corrupted to: 3000 abc............ The archive will load with a buffer overrun - a security risk. So then one should dynamically allocate the storage according to the size - that is one should be using the std::string . So I decided to comment out the code that handles char *. > > > " If all primitive types have been accounted for, any program with > > serialization defined should work with the new archive." > > > > Maybe I can expand upon that a little to something like: > > > > "Any program with serialization defined should work with the new archive > > as long as every primitive type has a matching save/load function > > prototype or template." > > Can I define only one 'load' for unsigned int? As oppose to ? They way I implemented the included archives I specified load for those requiring special treatment and used a template as a fall back for the rest. BTW this provided a huge benefit. In the original version of last year I got into a never-ending battle to specify virtual functions which was dependent on the compiler - long long, etc. It was hopeless - moving to templates solved that. > > > > I've concluded this myself. Its pretty easy given the bjam setup. > Maybe > > it can be made even easier with a bjam argument or environmental > variable. > > It > > just needs to be explained. > > Right. BTW as a bjam expert, you might want to suggest how to do this in cool way. I would love to have a shell script which lets me do test_archive <toolset> <archive> But I don't see how to fix up the bjam files to support this. As a bjam expert, maybe you could look into this. > Do you mean you plan to always serialize char* as arrays and remove > 'load'/'save' overloads for char*? Again, I'm not sure I understand the > motivation. See above Robert Ramey

2 1

Serialization - access control
by Matthew Vogt 22 Apr '04

22 Apr '04

Previously, I posted that I was unhappy with the way access to an archive's save methods could be controlled. This was because the class the eventually invokes the 'save' method of an oarchive is a library class, and has to be given friend access. I have now realised that this was trying to control access the wrong way; instead of making the type-specific 'save' method private, the type-specific overload of 'operator<<' should made private to prevent general access for saving a given type. Here is a basic test that shows what I had been trying to achieve, now working correctly: //--------------------------------------------------------------------------- #include <boost/serialization/access.hpp> #include <boost/archive/common_oarchive.hpp> class float_storer { friend class boost::serialization::access; private: float f; template<class Archive> void serialize(Archive& archive, const unsigned int version) { archive & f; } }; namespace archive = boost::archive; class simple_oarchive : public archive::common_oarchive<simple_oarchive> { friend class archive::common_oarchive<simple_oarchive>; friend class archive::save_access; // Do nothing with library internal types simple_oarchive& operator<< (const archive::class_id_type&) { return *this; } simple_oarchive& operator<< (const archive::class_id_optional_type&) { return *this; } simple_oarchive& operator<< (const archive::class_id_reference_type&) { return *this; } simple_oarchive& operator<< (const archive::class_name_type&) { return *this; } simple_oarchive& operator<< (const archive::version_type&) { return *this; } simple_oarchive& operator<< (const archive::object_id_type&) { return *this; } simple_oarchive& operator<< (const archive::object_reference_type&) { return *this; } // Save any type that gets here template<typename T> void save(const T& t) {} // Only allow 'float_storer' to store floats friend class float_storer; simple_oarchive& operator<< (const float& t) { archive::save(*This(), t); return *This(); } public: // Allow anyone to store ints simple_oarchive& operator<< (const int& t) { archive::save(*This(), t); return *This(); } simple_oarchive& operator<< (const float_storer& t) { archive::save(*This(), t); return *This(); } }; int main(int, char**) { simple_oarchive oa; int i; oa << i; float f; oa << f; // I want this to fail float_storer fs; oa << fs; // I want this to succeed return 0; }

1 1

Re: Re: Re: numeric_cast
by Dill, John 21 Apr '04

21 Apr '04

"Rob Stewart" <stewart(a)sig.com> wrote in message news:<200404211327.i3LDRov09314(a)lawrencewelk.systems.susq.com>... > From: "Dill, John" <john-dill(a)uiowa.edu> > > "Rob Stewart" <stewart(a)sig.com> wrote in message news:<200404202049.i3KKnwe25029(a)lawrencewelk.systems.susq.com>... > > > From: "Dill, John" <john-dill(a)uiowa.edu> > > > > "Rob Stewart" <stewart(a)sig.com> wrote in message news:<200404201905.i3KJ55e08992(a)lawrencewelk.systems.susq.com>... > > > > > From: "Dill, John" <john-dill(a)uiowa.edu> > > > > > > > > > > > > I am wondering about the use-case of numeric_cast in this sample. > > > > > > > > > > > > unsigned char uchar_max = std::numeric_limits<unsigned char>::max(); > > > > > > char schar_value = 0; > > > > > > > > > > > > try { > > > > > > schar_value = boost::numeric_cast<char>( uchar_max ); > > > > > > } > > > > > > catch ( boost::bad_numeric_cast ) > > > > > > { std::cout << "Overflow occurred..." << std::endl; } > > > > > > > > > > > > When I execute this sample, I don't get the exception. What's > > > > > > the background on this behavior? I'm using gcc 3.3.1. > > > > > > > > > > Your char is unsigned. > > > > > > > > I'd like to know why conceptually the exception isn't thrown. > > > > > > As I said, in too few words, is that your "char" is unsigned, so > > > "char" and "unsigned char" are essentially the same type. > > > Technically, they are distinct types (for overloading purposes, > > > for example), but they are both unsigned and the same size, so: > > > > > > assert(std::numeric_limits<char>::max() > > > == std::numeric_limits<unsigned char>::max()); > > > > > > on your system. > > > > Actually that is not the case. The max defined for unsigned > > char is 255, while for char is 127. Actually, char is > > supposedly equivalent to signed char, not to unsigned char. > > Based upon the evidence you presented, I concluded that your char > is equivalent to unsigned char. On what basis do you conclude > that your char is equivalent to signed char? Did you check > std::numeric_limits<char>::is_signed? Yes. Both char and signed char have identical is_signed and max, min. Unsigned char min, max and is_signed is different. Does the standard say that char can be either signed char or unsigned char, or is that just an implementation issue? Best, John

3 2

Re: numeric_cast
by Dill, John 21 Apr '04

21 Apr '04

"Fernando Cacciola" <fernando_cacciola(a)hotmail.com> wrote in message news:<c64iag$bes$1(a)sea.gmane.org>... > > "David Abrahams" <dave(a)boost-consulting.com> escribió en el mensaje > news:u3c6ywa5w.fsf@boost-consulting.com... > > "Dill, John" <john-dill(a)uiowa.edu> writes: > > > > > I am wondering about the use-case of numeric_cast in this sample. > > > > > > unsigned char uchar_max = std::numeric_limits<unsigned char>::max(); > > > char schar_value = 0; > > > > > > try { > > > schar_value = boost::numeric_cast<char>( uchar_max ); > > > } > > > catch ( boost::bad_numeric_cast ) > > > { std::cout << "Overflow occurred..." << std::endl; } > > > > > > When I execute this sample, I don't get the exception. What's the > > > background on this behavior? I'm using gcc 3.3.1. > > > > FWIW, there are known problems with numeric_cast, but a numeric > > conversion library that replaces the existing implementation has been > > accepted into Boost. We are just waiting for Fernando Cacciola to > > check it in. Fernando? > > > Indeed. The accepted numeric conversions library handles this case properly > (throwing). > Right now I'm technically on vacation until next Monday. I'll start working > with the review comments right next week. I'll check it in on CVS after I > made the neccesary changes. > > Anyway, users can download the review version from here: > > http://personales.ciudad.com.ar/fernando_cacciola/numeric_conversions.zip > > or > > http://groups.yahoo.com/group/boost/files/numeric_conversions.zip > > > Fernando Cacciola Thanks. Well, I didn't know this was out there so I wrote my own numeric_cast which works for those cases I described. It's not boost oriented, but all of the concepts should be convertible to mpl. Maybe it will be useful to someone. Best, John /*! * \file numeric_cast.hpp * \brief A cast which protects against silent underflow or overflow casts. */ #include <uiro/type/ct_if.hpp> #include <uiro/type/typelist.hpp> #include <uiro/utility/limits.hpp> namespace uiro { //! Exception used to indicate underflow in a numeric cast. class bad_numeric_cast : public std::bad_cast { public: virtual const char* what() const throw() { return "bad numeric cast"; } }; //! Exception used to indicate underflow in a numeric cast. class numeric_cast_underflow : public bad_numeric_cast { public: virtual const char* what() const throw() { return "underflow in numeric_cast"; } }; //! Exception used to indicate overflow in a numeric cast. class numeric_cast_overflow : public bad_numeric_cast { public: virtual const char* what() const throw() { return "overflow in numeric_cast"; } }; namespace internal { //! Selects the type with the largest minimum value. template <typename T1, typename T2> struct promote_min_traits { typedef TYPELIST_10(unsigned char, unsigned short, unsigned int, unsigned long, char, short, int, long, float, double) promote_order_typelist; enum { index1 = uiro::type::index_of<promote_order_typelist, T1>::value }; enum { index2 = uiro::type::index_of<promote_order_typelist, T2>::value }; typedef typename uiro::type::ct_if< uiro::type::greater_than_equal<index1,index2>::result, T1, T2>::result result; }; //! Selects the type with the largest maximum value. template <typename T1, typename T2> struct promote_max_traits { // May run into issues if sizeof(int) != sizeof(long) typedef TYPELIST_10(char, unsigned char, short, unsigned short, int, long, unsigned int, unsigned long, float, double) promote_order_typelist; enum { index1 = uiro::type::index_of<promote_order_typelist, T1>::value }; enum { index2 = uiro::type::index_of<promote_order_typelist, T2>::value }; typedef typename uiro::type::ct_if< uiro::type::greater_than_equal<index1,index2>::result, T1, T2>::result result; }; struct unsigned_tag {}; struct signed_tag {}; //! A numeric cast helper which handles the an unsigned source type //! without generating compiler warnings. template <typename Target, typename Source> inline Target numeric_cast_helper( Source arg, unsigned_tag ) { // An overflow condition. if ( static_cast<typename promote_max_traits<Source, Target>::result>( arg ) > static_cast<typename promote_max_traits<Source, Target>::result>( std::numeric_limits<Target>::max() ) ) { throw numeric_cast_overflow(); } return static_cast<Target>(arg); } //! A numeric cast helper which handles the an signed source type. template <typename Target, typename Source> inline Target numeric_cast_helper( Source arg, signed_tag ) { // A loss of negative range, or more generally an underflow. if ( ( arg < 0 && !std::numeric_limits<Target>::is_signed ) || ( static_cast<typename promote_min_traits<Source, Target>::result>( arg ) < static_cast<typename promote_min_traits<Source, Target>::result>( std::numeric_limits<Target>::min() ) ) ) { throw numeric_cast_underflow(); } // A loss of positive range. if ( static_cast<typename promote_max_traits<Source, Target>::result>( arg ) > static_cast<typename promote_max_traits<Source, Target>::result>( std::numeric_limits<Target>::max() ) ) { throw numeric_cast_overflow(); } return static_cast<Target>(arg); } } // end of namespace internal //! A numeric cast which throws on overflow or underflow. template <typename Target, typename Source> inline Target numeric_cast( Source arg ) { typedef typename uiro::type::ct_if<std::numeric_limits<Source>::is_signed, typename internal::signed_tag, typename internal::unsigned_tag>::result tag; return internal::numeric_cast_helper<Target>( arg, tag() ); } } // end of namespace uiro

1 0

Re: Re: numeric_cast
by Dill, John 21 Apr '04

21 Apr '04

"Rob Stewart" <stewart(a)sig.com> wrote in message news:<200404202049.i3KKnwe25029(a)lawrencewelk.systems.susq.com>... > From: "Dill, John" <john-dill(a)uiowa.edu> > > "Rob Stewart" <stewart(a)sig.com> wrote in message news:<200404201905.i3KJ55e08992(a)lawrencewelk.systems.susq.com>... > > > From: "Dill, John" <john-dill(a)uiowa.edu> > > > > > > > > I am wondering about the use-case of numeric_cast in this sample. > > > > > > > > unsigned char uchar_max = std::numeric_limits<unsigned char>::max(); > > > > char schar_value = 0; > > > > > > > > try { > > > > schar_value = boost::numeric_cast<char>( uchar_max ); > > > > } > > > > catch ( boost::bad_numeric_cast ) > > > > { std::cout << "Overflow occurred..." << std::endl; } > > > > > > > > When I execute this sample, I don't get the exception. What's > > > > the background on this behavior? I'm using gcc 3.3.1. > > > > > > Your char is unsigned. > > > > I'd like to know why conceptually the exception isn't thrown. > > As I said, in too few words, is that your "char" is unsigned, so > "char" and "unsigned char" are essentially the same type. > Technically, they are distinct types (for overloading purposes, > for example), but they are both unsigned and the same size, so: > > assert(std::numeric_limits<char>::max() > == std::numeric_limits<unsigned char>::max()); > > on your system. Actually that is not the case. The max defined for unsigned char is 255, while for char is 127. Actually, char is supposedly equivalent to signed char, not to unsigned char. > > Why is it beneficial to have unsigned T to signed T overflows > > not be detected? It is not just char, but short, int, and long > > as well. > > Now that's another story. Changing "unsigned char" to > "unsigned" and "char" to "int" should cause an exception. If > that's not happening, I don't understand since > std::numeric_limits<unsigned>::max() is greater than > std::numeric_limits<int>::max(). Actually, I think those type conversions shouldn't cause an exception. Converting char to int for example should be no problem since the range of char is within the range of int. Should be similar for unsigned char and unsigned. > > If this was the intended behavior, then what is the reasoning > > behind it? > > What you describe isn't the intended behavior other than the char > problem. I think it is a problem when you have an unsigned char, range 0 to 255, put into a char, range -128 to 127, when you are converting an unsigned char value of 255 (the numeric_limits::max()) into a [signed] char. It is an overflow problem, and it doesn't generate an exception. From what you seem to say, this should be a bug. Best, John

2 1

Re:[boost] Formal review: serialization
by Robert Ramey 21 Apr '04

21 Apr '04

Vladimir Prus wrote: > Detailed comments are below. Some of them are marked as "critical" -- it > means I believe they should be addressed before the library is added to > official distribution. > > ** General issues ** > > -- (critical) The primary issue with the library is lack of reference > documentation -- exact description of semantics for every method > (preferrable in C++ std style) and overview for all important classes. The > library is non-trivial, and without such documentation user will have to > experiment to find out that exact semantic. > > Better internal docs (comments) are also needed. For example, I'm looking > at > save_access::end_preamble and don't know what it does. Comment for > basic_oarchive::end_preamble says what this method is for, but does not > say > when this method is called, which makes it hard to do anything. Again, > implementation is non-trivial. Lack of comments make it somewhat dangerous > to > use the library -- if I hit a bug while Robert is on vacation, I'm not > sure > I'll be able to quickly fix it locally. > > As an illustraction, I've already had a couple of bugs each costing > several hours to debug. > I think this is a little harsh. I've had lots of things I had spend several hours on - including iterator_adaptors, spirit, and mpl. I'm not against adding things to the documentation but in general I think it as good as or better than any other boost documentation. Having said that, I do incrementally clarify bits of documentation as people ask me questions. There's another point that has me a little bit stumped. Most of the boost libraries are code that one invokes with a documented signature. The list of invocations, syntax and semantics is what I think Vladimir is referring to here with "reference documentation". However, the serialization library is more "upside down" in that using it consists almost entirely of filling in stubs called by the serialization algorithm. So its not clear to me how the "reference documentation" would look compared to the current documents. > -- (critical) While BOOST_CLASS_EXPORT is a big step forward, I think it > can > be further improved. Now, if I export a class I need to include some > archive > headers before invoking BOOST_CLASS_EXPORT. The class can be only > serialized > to archives which were included. This means that to really export a class, > I > need to include *all* archive headers. That's not good. What I suggest is > > - create new class 'polymorphic_archive', which will have virtual > functions for saving/loading and create templated > 'polymorphic_wrapper', which can be used to add this polymorphic > interface to existing archive. > - always register all exported classes with polymorphic archive. When > serializing in a specific archive, check if class is registered > with > that archive. If not, serialize it via polymprphic wrapper. > > In fact, I'd be willing to help with polymorphic archive implementation, > but > some changes inside serialization library will be required. > There was much objection to the previous version which was based on virtual functions. The usage of templates has addressed those concerns but raised others related to instanticatiation. In the previous system the virtual function interface was fixed across all archives (even those not invented yet !) in the current system serialization code is generated from the archive and serialization templates a new every time. So as it stands one can't make a "library for type X" for all future archives. This can be addressed in two ways. For current archives one can follow the demo_pimpl example, and generate a library that has pre instantiated code for all know archives. This will be satisfactory in many cases. For those cases not addressed by the above solution - i.e. those a virtual interface will be required. This can be addressed by creating of a "polymorphic_archive" without requiring changes to the internals to the library. This is not difficult if done carefully, but it hasn't been done yet. It entails definition of an interface class. The concrete archive class would inherit and from both this interface as well as one of the current implementations. Its exactly the way the Microsoft COM is implemented. I have yet to do this. > -- (critical) I suspect BOOST_CLASS_EXPORT does not play nice with > templates. > Say I'm implementing RPC system: > > class function_call_base { virtual ~function_call_base() {} } ; > template<class Arg> > class function_call1 : public function_call_base {}; > > // in some code > rpc_server s; > s.register("foo", &foo); > > function_call_base* c; > while(archive >> c) > (*c)(); > > The idea is the caller will serialize instances of function_call1 and the > callee will deserialize it and call the proper function. To make this > work, > we have to BOOST_CLASS_EXPORT all used instantinations of function_call1. > We > can assume that all functions (and therefore all argument types) are > passed > to the 'register' function and can function can invoke BOOST_CLASS_EXPORT. > > Now the problem: BOOST_CLASS_EXPORT can be used only at top-level of a > translation unit. There's not way 'register' and automatically export a > class. > > The solution I'd like is: > > template<class T> > class export_class { > public: > static void instantiate(); > }; > > So that the object of this type can be created anywhere. > Doesn't including BOOST_CLASS_EXPORT in the header fill corresponding to the exported data type perform the equivalent function? It was certainly my intention that that was how it would be used. In this way the "export" would be addressed once and for all. That way all types used - and only those types would be instantiated. > -- One usage of the serialization library I have in mind is taking "debug > dumps" for an application. After the run, a separate tool can be used to > look > at those dumps. The size of data can potentially be rather large (e.g. > 100*640*480 or maybe more), so I'd rather not to load all data, but build > some index. So the question is: how is it possible to access specific part > of > archive without reading it all. I understand it's not possible in general, > but in some cases (e.g. BOOST_CLASS_EXPORT for everything) it should work. > I think the could fairly easily be addressed by making a derivation from the desired archive type and making an index on class_id and object_id or something like that. I wonder if there isn't an XML reader that opens just the toplevel and only opens the file as one "drills down". Then just serialize to XML and use that reader. > -- The documentation of 'make_binary_object' is very incomplete and does > not > tell the exact semantic. As far as I can tell, the function can be only > used > to load/store objects which have fixed size and will crash when > saving/loading new[]-d array -- which seemed an obvious use-case for me. > > I would suggest that we have two functions: > > make_pod_wrapper -- which is templated, has one parameter -- object, > and > stores sizeof(T) bytes. Another name -- make_static_array_wrapper. > > make_dynarray_wrapper -- which can be used to store/load arrays > allocated > by new[] > I can expand the doc a bit. binary_object is just a wrapper around a size and pointer to permit them to be handled as pair. It presumes the pointer already points to allocated storage. It also presumes that the the size of thing its pointing to is the same on save and load. Its very lightweight. I don't see a need for the others, but of course any user can make the wrappers he needs. > -- The library requires exact match between static types for saving and > loading. For example: > > D* d = new D; > ia << d; > B* b = new B; > ia >> b; > ( presume you mean don't mean "new B" as the library creates a new B) > does not work. Moreover, it does not work even if 'D' is > BOOST_CLASS_EXPORT-ed. I'd suggest that this case should work, since it's > very easy to save derived classes using pointer to derived class, without > casting to base class. It doesn't work. Generally saving to one type and loading to another is not supported. In this particular case it could be made to work but it would significantly complicate the library. Also, supporting a practice such as the above, would lead to serialization which is much harder to debug. I don't believe that there is a situation in which doing the above is necessary. The appropriate coding of the above would be: B* b = new D; ia << d; B* b2; ia >> b2; I don't think that's an unreasonable burden. > -- It appears that XML archive require member/variable names in all cases. > I'm > not sure that's reasonable -- given that the library requires exact > correspondence between loading and saving order even for XML archives. It > would load archive back even if I change all the names manually in XML > file > right? Then why requiring names? > > -- I have some concerns about XML archives. The point of XML, in general > is > that it's somewhat readable by humans, and readable by other applications > written using different libraries or in different languages. However, when > I > look at: > > <c2 class_id_reference="1" object_id="_2"> > <i>10</i> > <j>4</j> > <base object_id="_3"></base> > </c2> > > it's not clear if any of those benefits are there. This actually > corresponds > to a class "C". To find out this, one has to find "class_id" attribute > with > value of "1" somewhere above. If the above was: > > <C name="c" ...> > > finding all "C" elements would be trivial with XPath, or even with more > simple > methods. As it stands now, even finding all instances of "C" is hard. > There > may be a number of similiar issues. If the goal is to produce XML which > can > be processed by other programs, then at least the class name should become > element name. > Correct, the names aren't used on input. The real question here is what XML is to be used for. Personally I have no use for it. I did it to satisfy boost members who thought it was indispensable. After studying the options in more detail and learning more about it, I'm even more convinced its utility is way over hyped. The question is what should be included in the attribute list. I made my choices and no one has complained. I didn't put anything in there that wasn't necessary for de-serialiation except the name. (Do you think those requiring XML would be satisified if it didn't have the name?). The export class name is included if its necessary. The XML attribute list could be tweaked a little without too much trouble. But it really comes down to what is XML serialization going to be used for besides de-serialization. Until that's answered, I think this will have to remain an open question. As its stands, I looked at, made my choices and hardly anyone has complained. I guess that its probably has limited utility. I think that everyone has assumed that someone else will find it indispensable. BTW, one usage of XML archives did occur to me. By checking the name tag on input, we can implement a crude check that save and load operations are synchronized. This would effectively a debug mode for archives and might be useful. > -- I believe that library should not change codecvt facet of the stream. > There > are two reasons. First, if user has already imbued some facet (say utf-8), > he > might have good reasons to do so (say, he believes utf-8 is the most > space-efficient for his data). So the serializaton library should not > silently change codecvt to ucs-2. Second, the whole locale stuff is > complex, > and it's not even clear if it's possible to change codecvt facet on the > fly, > and when the change will take effect, and so on. I believe requiring user > to > 'imbue' proper facet himself is better than some unexpected internal > effects. > I've wrestled with that. My current thinking is that the system should not be changed. The reason I did this was that I found that the default codecvt on wchar_t files was to narrow the character and throw away the high order bits. Also UTF-8 is need for wchar_t XML. I included the ability to override this by specifying no_codecvt flag upon opening the archive which leaves the codecvt unchanged. So I'm happy with the current method which is to reflects the most common user intention and permits one to suppress this default behavior. I've come to conclude that that the only situation where the codecvt can't be changed on the fly is with the old dinkumware library. I'm still checking for this though. > -- I've tried to play with "implementation level" and created the example > at > > http://zigzag.cs.msu.su:7813/tracking.cpp > > It stores a pointer to a single class with two data members and a single > base > class. I get the following archive content. > > 1 > 1 C > 0 10 4 1 0 > 1 > > Is seems too much for a class with implementation level of "object > serializable". 10 and 4 is the data itself. "1 C" is probably class name, > but > what other data is? The easiest way to verify this is to use an XML archive which labels all the items. Current archives contain no extraneous data (except the name in the XML archive). <?xml version="1.0" encoding="UTF-8" standalone="yes" ?> <!DOCTYPE boost_serialization> <boost_serialization signature="serialization::archive" version="3"> <c class_id="1" class_name="C" object_id="_0"> <i>10</i> <j>4</j> <base class_id="0" tracking_level="1" object_id="_1"></base> </c> </boost_serialization> > > ** Implementation issues ** > > -- I suggest that all MPL expressions in the library are refactored for > better > readability. For example, the expression in save_non_pointer_type::invoke > is > 28 lines long. I think it should be possible to extract some of the > subexressions and create a typedef with good name -- looks like I think the indentation is better than creating new names. > instantination of those expression in all cases should not cause compile > errors. Hmm - please expand on this. > > ** Custom archive creation ** > > I've tried to create a custom archive class (in fact, > polymorphic_oarchive), > and got some comments. > > - Documentation says the derived class must provide operator<< and method > 'load'. It also says there common_iarchive::operator<<. I wonder why the > derived class should provide operator<<. Can't base operator call 'load' > directly? > > - Even if not, why derived operator<< should have > > return * This(); > > not plain "return *this"? After all, we're in most derived object > > - It's needed to add > > friend class boost::serialization::load_access; > > to class declaration. However, it's not stated where this type is > declared, > and in fact in declared in *internal* header I wasn't aware that such a friend class had to be declared. Recently one compiler has flagged with a warning which is annoying. So, we'll look at this if only to suppress a benign warning. > > boost/archive/detail/iserializer.hpp > > And, finally, the right syntax seems to be: > > friend class boost::archive::load_access; > > (Note the namespace!) > Hmm - I don't see that this is required given that its be called from within boost::archive::detail. Does any compiler complain? > - The example uses: > > boost::serialization::load(* This(), t); > > Again, what header is this function in? The answer is that it's in the > same > header as above, and the line should be: > > boost::archive::load(* This(), t); > Correct thanks. > - The documentation should really state the minimal set of 'load'/'save' > overloads which will make archive usable. For example, it's probably not > necessary to provide separate overload for 'bool', right? The document states: " However, we're not quite done. The above code addresses serialization of all non-primitive types. To be complete, each primitive type must either be covered by a definition of template<typename T> void load(T & t); or an overload of the >> operator" And " If all primitive types have been accounted for, any program with serialization defined should work with the new archive." Maybe I can expand upon that a little to something like: "Any program with serialization defined should work with the new archive as long as every primitive type has a matching save/load function prototype or template." The trivial_?archive include the "fall-through" template that matches any primitive type - including bool. For example, the "fall-through" for text_oarchive is Void load(T &t){ os >> t; } Which covers all primitive types. > - (critical) It should be explained how the newly-created class can be > tested. > As it stands, one should either do superficial tests or try to integrate > the > new archive into existing tests. As for latter -- I don't know how to do > that, and also, I think it's not necessary to run all the tests with the > new > archive. For example, serialization of vector and map is not likely to be > broken with the new type. What I'm looking for is a test which saves/loads > all possible types and verifies results. > I've concluded this myself. Its pretty easy given the bjam setup. Maybe it can be made even easier with a bjam argument or environmental variable. It just needs to be explained. The easiest is to just run all the tests - for one archive it probably takes less than 30 minutes and its fairly exhaustive. So its not worth trying to figure out a way to abbreviate the test. (Not to mention the fact that the test I skip is always the one that catches me). In some situations the brain-dead way is really the best way. > ** Various comments ** > > -- The BOOST_CLASS_EXPORT macros defines an explicit instantiation of a > function. gcc rejests this when BOOST_CLASS_EXPORT is used inside some > namespace. I've asked on comp.std.c++: > > http://groups.google.com/groups?q=Explicit+instantiation&hl=en&lr=&ie=UTF- > 8&safe=off&selm=u5boi1-7te.ln1% > 40zigzag.lvk.cs.msu.su&rnum=3 > > and got a reply directing me to: > > http://std.dkuug.dk/jtc1/sc22/wg21/docs/cwg_defects.html#275 > > Which means gcc is mostly right. So, the macro should either be changed > somehow, or at least the restriction should bes documented -- using > BOOST_CLASS_EXPORT outside of any namespaces is OK. > I'll add language reminding users that ALL these macros have to be invoked outside of any name space. Hmmm > -- Docs and code use "const unsigned long int version" everywhere. Is that > really necessary, as opposed to "unsigned version"? That's been fixed. I'm not sure you have version # 18. > > -- The library saves char* as arrays of integers. The rationale section > says: > > input of char * and wchar_t * could be altered to create a buffer > overrun > and potential security problem. > > I don't understand this point at all. How storing the elements as integer > values helps with the buffer overrun? Moreover, library stores char* > internally (e.g. for archive signature). What if that string is modified? > Will that cause buffer overrun? The manual is wrong. I'll fix it. I commented out the code that serializaed (char *) and (wchar_t *) as strings. I was concerned about buffer overrun security problems. I don't remember my reasoning for this right now but I don't think implemention char * serialization as a primitive type is a good idea. > > -- The following program (yes, of two lines), causes a compile error: > > #include <boost/archive/text_oarchive.hpp> > #include <boost/serialization/export.hpp> Not on my machine. So I guess I fixed it. Whoops, fails on gcc - so I've fixed with a forward template declaration. Robert Ramey

3 3

program_options suggestion
by Neal D. Becker 21 Apr '04

21 Apr '04

I really like program_options from Vladimir Prus. Thanks for adding bool_switch! One suggestion. We now have bool_switch, so a variable can default to off, and turn on with --option (without having to say --option=true). How about begin able to say --no-option to turn it off? This would make the interface consistent with gnu standards. If it's not too difficult it would make a good addition.

2 1

Re: Re:[boost] Formal review: serialization
by Robert Ramey 21 Apr '04

21 Apr '04

I just realized I skipped something Vladimir Prus wrote: > > ** Custom archive creation ** > > I've tried to create a custom archive class (in fact, > polymorphic_oarchive), > and got some comments. > > - Documentation says the derived class must provide operator<< and method > 'load'. It also says there common_iarchive::operator<<. I wonder why the > derived class should provide operator<<. Can't base operator call 'load' > directly? > common_iarchive has no operator<< 'load' is called by the library to load primitives - its generally not called by the << operator unless one wants to bypass the serialization system entirely. Perhaps, my usage of load_override was confusion. I use load_override in order to work around the lack of partial template function ordering. Were it not for this, load_override would not exist. This is an implementation artifact. Load on the other hand is the function used to load primitives. > - Even if not, why derived operator<< should have > > return * This(); > > not plain "return *this"? After all, we're in most derived object > No we are not. The class hierarchy is common_iarchive base_text_oarchive text_oarchive_impl text_oarchive in my archives, the operator<< is a member of base_text_oarchive. If I want the << operator to return a reference to a text_oarchive NOT basae_text_oarchive. The archives included are models of the trivial_archive. However, the desire to factor things out in a convenient way makes them less transparent than I would prefer. Robert Ramey

1 0

Re: [boost] Re: Using Serialization for binary marshalling
by Robert Ramey 21 Apr '04

21 Apr '04

Matthew Vogt wrote: >Robert Ramey <ramey <at> rrsd.com> writes: >> Correct. That's exactly what the current implementations of STL classes >> do. So my question is why override vector for you xdr/cdr archives? > Primarily, because the implementation you have chosen for default > serialization of STL containers is just an implementation detail from your > point of view; you could quite reasonably change it in the next version, > if someone with alternate requirements requested that, and it didn't > conflict with your own purposes. OTOH, XDR and CDR mandate how sequences > should appear in a stream, so it is necessary to ensure that the default > implementation is not relied upon for conformance. OK, but then why not just delay the effort until the underlying implementation actually changes? > Another reason, is that other formats will vary the implementation in > minor ways - by using a preceding length field in 8-bit signed format, for > example. I believe this could be addressed without too much trouble - perhaps by making a special type (a strong type) to hold collection count and tweaking collection implementation to use it. So it could then be implemented to taste on each kind of archive. >> I would like to see your xdr/cdr archives finished off at the same >> level of generality as the other archives. Any special purpose >> marshalling would leverage off that. > I don' think this is possible, since these formats do not specify > Sufficient types to do so. The library maps an arbitrary C++ data structure to a sequence of C++ primitives and back again. Any format that can handle all C++ primitives can therefore be used to serialize any C++ data structure. I'm sure that XDR/CDR can represent any C++ primitive data type. Hence, XDR/CDR can be used to serialize any C++ data structures with this library. > It would be possible to allow type promotion > to cover some missing types - for example, in XDR, it would be possible to > allow all C++ integral types less than 32-bits to be promoted to 32-bits > during serialization, but I think this would be misleading. Misleading? > More problematically, there is no concept of objects in this situation. Of course at the archive level you only see a string of numbers. It's the serialization that recovers C++ objects from all of this. > It wouldn't make sense to transmit objects via pointers, since 'the other > side' will never reconstitute objects. Perhaps this is a source of confusion. It never occurred to me that the other side wouldn't have the mirror serialization code. That is of you're using xdr_oarchive to make the stream I assumed you would use xdr_iarchive on the other side to recover the data. Even if you don't anticipate the need for doing this yourself, other users will and they will want the universal coverage that the current system provides. I wager that if you tell users on this this list - "Oh the XDR archive is special, it's just for marshalling, you can't do objects with XDR" you'll get an earful. And its not necessary to subject oneself to this kind of grief. It's much less work to provide for every C++ primitive data type and know that any serialization is always going to work than it is to try to anticipate ahead of time for just those combinations of features users are going to actually use. If you want a "restricted" or special purpose xdr archive that prohibits pointers or something else in order to conform to some other program not built with this library, you can just derive from the simple general one and overload the prohibited operations with compile-time assertions. Robert Ramey

3 2

a question about "typeof"
by Arkadiy Vertleyb 21 Apr '04

21 Apr '04

Hi all, A couple of questions regarding typeof: 1) Is it going to be in the next C++ standard? 2) Is there any estimate when it is going to happen? I assume that quite a few Boost libraries could largely benefit from this operator. Spirit documentation even contains a section on typeof and how the syntax could be improved with it. I recently had a need to determine a type of a Lambda functor (but got away with Boost.Function with some performance expences). Also Joaquin recently raised a similar question. Our RTL syntax would also benefit a lot if this facility was available. About one and a half year ago Steve Dewhurst published three articles on this subject in CUJ. The idea was to encode the type as an integer using bitwise operations. This integer could be than "returned" from the function, this crossing the boundaries between a type and a value, and then decoded into the original type. I wonder if this or any other technique is currently being considered by anyone to be added to Boost or we are just waiting until the real typeof is available? I have been experimenting with this, and came up with a very simple trick that allows to use a static list of integers, like mpl::vector_c, instead of bitwise operations, to encode the type, and pass this list across the function boundaries. I think, if nothing else, this is much simpler than bitwise operations. Everything else is pretty much like in Steve's approach. Based on this it seems to be pretty simple to encode/decode types to implement typeof-like facilities for some specific (or maybe not so specific) cases. Has anybody already done something similar, and/or is this of any interest? If there is an interest I am ready to provide explanation, code, etc. Regards, Arkadiy

5 5