Re: [boost] Input archive loads seem to corruptsharedcharacterstd::strings

From: Martin Bonner (Martin.Bonner_at_[hidden])
Date: 2007-12-04 06:19:33

From: Hervé Brönnimann

>> On Dec 4, 2007, at 5:13 AM, Jens Seidel wrote:
>> To be honest, isn't a specific implementation not completely
>> unimportant? You don't want to rely on implementation details, right?
> 
> I don't rely on gcc's string implementation details, but on whether a
> string implementation is shared or not.  

But whether a string is shared or not is an implementation detail.
> The original problem and
> argument I built upon it is true for any shared implementation
... that you can think of.  I bet I can imagine an architecture where the behaviour is different.

> If
> you think there's no way in C++ to tell whether a string class is
> shared or not, think about Chuck's posting:
> 
> bool is_string_class_shared() {
>      const std::string s1('x');
>      const std::string s2(s1);
>      *(const_cast<char*>s1.data()) = 'y';

At this point you have invoked undefined behaviour, so it is impossible to use the C++ standard alone to reason about the behaviour of the code.  

>      return s2[1] == 'y';
> }
> 
> Speaking of the programming with only the C++ standard, it
> specifically mentions allowing a shared implementation of std::string.
Yes, and puts certain restrictions on what a programmer can do so that a shared implementation of std::string can work.  (For example, a programmer may not write the code in "is_string_class_shared".)

>> There is only one solution: remove the buggy casting code and
>> replace it with a proper one as suggested. Whether the proper
>> one is slower or not is unimportant!
> 
> I strongly disagree.  Performance of string serialization is
> important, as Robert posted in a previous msg.  For that reason, the
> use of const_cast<char*>s.data() seems justified.

This is the heart of the matter.  What is the difference in performance between relying on the const_cast, and the strictly legal version using a vector buffer:

text_iarchive_impl<Archive>::load(std::string &s)
{
    std::size_t size;
    * this->This() >> size;
    // skip separating space
    is.get();

    // Read into a buffer.
    std::vector<char> v(size);
    is.read( &v[0], size );

    // Return the string.
    std::swap( s, std::string( v.begin(), v.end() ));
}

Clearly there are inefficiencies there, but are they bad enough to be worth the const_cast?

[snip] 
>> No, the problem is that this is implementation specific. Don't rely
>> on such stuff, rely on the C++ standard!
> 
> Again, my argumentation isn't implementation specific, it's true of
> any shared string class.
Which is implementation specific.

> My patch should work with any standard-compliant string.  
I don't think so.  (Although it may well work with any /plausible/ implementation of a standard compliant string).

> What I
> argued was that it's better than the previously proposed fixes for
> gcc's implementation.  
Ah.  Now that's a /very/ different argument :-)

-- 
Martin Bonner
Senior Software Engineer/Team Leader
PI SHURLOK LTD
Telephone: [phone] / 203894 (direct)
Fax: [phone]
Email: martin.bonner_at_[hidden]
www.pi-shurlok.com
disclaimer