David Abrahams wrote:
I can't imagine how your change would fix an endianness problem.
It does not fix "an endianness problem". The bug is in the code conditionalized with '#ifdef BOOST_BIG_ENDIAN'. This is the only connection between endianness and the fix.
Also, your change relies on the nonportable (implementation-defined) behavior of reinterpret_cast.
It does. As well as other Boost code using reinterpret_cast :-) Scanning Boost sources (trunk) for reinterpret_cast yields 700 hits, including 8 instances in the Boost python library (two of them are commented out): bash-2.03$ find ./boost/libs/python -type f -exec grep reinterpret_cast {} \; instance<>* self = reinterpret_cast<instance<>*>(inst); _doc += str(reinterpret_cast<const char*>(detail::py_signature_tag)); // _doc += "\n"+str(reinterpret_cast<const char*>(detail::cpp_signature_tag)); _doc += str(reinterpret_cast<const char*>(detail::cpp_signature_tag)); instance<>* self = reinterpret_cast<instance<>*>(inst); _doc += str(reinterpret_cast<const char*>(detail::py_signature_tag)); // _doc += "\n"+str(reinterpret_cast<const char*>(detail::cpp_signature_tag)); _doc += str(reinterpret_cast<const char*>(detail::cpp_signature_tag)); bash-2.03$ You are, probably, referring to 5.2.10 - Reinterpret cast [expr.reinterpret.cast], para 3: -3- The mapping performed by reinterpret_cast is implementation-defined. [Note: it might, or might not, produce a representation different from the original value. ] In my fix, reinterpret_cast performs a mapping from non-null pointer to long to pointer to char. If reinterpret_cast cannot be used in this case without invoking implementation-defined behavior, I'm not sure how it can be used in a multiplatform code like Boost at all.
I think the correct change to silence the type conversion error is
char * first = static_cast<char *>(static_cast<void *>(const_cast<long*>(& l)));
I'm not sure my change is incorrect. In terms of style, I think that your change is better. Thanks, Boris ----- Original Message ----- From: "David Abrahams" <dave@boost-consulting.com> To: <boost@lists.boost.org> Sent: Friday, November 02, 2007 4:33 PM Subject: Re: [boost] [serialization] patch for portable_binary_oarchive.hpp
on Fri Nov 02 2007, "Boris Gubenko" <Boris.Gubenko-AT-hp.com> wrote:
Two serialization library tests: test_demo_portable_archive[_dll] fail to compile on many platforms with invalid type conversion error. I did not check all the platforms, but I'd expect it to fail on any big endian platform.
I can't imagine how your change would fix an endianness problem. Also, your change relies on the nonportable (implementation-defined) behavior of reinterpret_cast.
I think the correct change to silence the type conversion error is
char * first = static_cast<char *>(static_cast<void *>(const_cast<long*>(& l)));
However the code in itself invokes undefined behavior by modifying a constant object (l). The byte reversal needs to be done in a separate buffer.
The patch below fixes the problem, verified on HP-UX with both aC++ and gcc.
Ok to commit?
Index: example/portable_binary_oarchive.hpp =================================================================== --- example/portable_binary_oarchive.hpp (revision 40687) +++ example/portable_binary_oarchive.hpp (working copy) @@ -76,7 +76,7 @@
// we choose to use litle endian #ifdef BOOST_BIG_ENDIAN - char * first = static_cast<char *>(static_cast<void *>(& l)); + char * first = reinterpret_cast<char *>(const_cast<long *>(& l)); char * last = first + sizeof(l) - 1; for(;first < last;++first, --last){ char x = *last;
Thanks, Boris
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
-- Dave Abrahams Boost Consulting http://www.boost-consulting.com
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost