Boost logo

Boost :

From: Vladimir Prus (ghost_at_[hidden])
Date: 2005-05-11 02:01:27

Robert Ramey wrote:

>>>>> ofstream ofs(av[2]);
>>>>> boost::archive::binary_oarchive oa(ofs);
>>>>> oa << *data_builder.finish();
>>> This code should be considered erroneous. The documentation in 1.32
>>> addressed this but unfortunately the enforcement was lost in the
>>> shuffle.
>> Excuse me, but I refuse to accept that this code is erroneous. As
>> I've said, this is my existing design -- the 'finish' function
>> returns non-const pointer. The fact that somewhere else, I serialize
>> the result of 'finish'
>> has nothing to do with 'finish' return type, and I should not be
>> forced to change my existing interface just because I use
>> serialization somewhere.
> OK let me rephrase. This code conflicts with the description of the
> description of the functioning of the operator << as described in the
> documentation.

Can you give me specific doc URL? I can't find the relevant part in TOC.

>> So, you're saing that "const" == "no allocated on stack"? I don't see
>> why this statement is true. I can just as well do this:
>> void foo(const A& a)
>> {
>> ar << a;
>> }
>> and circumvent your protection.
> The intention is that const indicates that the process of serialization
> will
> not change the object being serialized. Serializing and object that
> cannot
> easily be passed as a const is quite possibly an error. Of course, the
> contrary is not necessarily true. That isan object

What's "isan"?

> can be passed as const
> referene and still be modified during the process of serialization. So
> this is not bullet proof - but I believe it is very helpful.

Let me draw an analogy: there's std::ostream. For years, we save objects to
ostreams and we're not required to save "const" object. The fact that
serialization acts in a different way is inconsistent and confusing. I
think you'll agree that users should not read documentation just to save an

And let me clarify again -- is this indended to stack only stack allocated

>>> Note that the documentation suggests that he above be reformulated as
>>> For(...
>>> ar << a[i]; //
>>> Enforcing const-ness also has the effect of preventing serialization
>>> from altering the state of the object being serialized - another
>>> almost impossible to find bug.
>> Can you explain:
>> 1. How this bug can happen
> Its very easy to write
> for(...{
> X x = *it; // create a copy of
> ar << x
> }
> all the x's are at the same address so if they happen to be tracked
> because a pointer to some X in serialized somewhere in the program, then
> the
> subsequent copies would be supprsed by tracking.

So, you're saying that the behaviour of the above code will be different,
depending on whether some pointer to X is saved somewhere else? This is
very strange -- here, saving is not done via pointer, so I expect that no
tracking is ever done. I'd argue this is a bug in serialization.

The only problematic case is

     X x = *it; // create a copy of
     ar << x

 X* x = new X:
 ar << x;

where address of newed 'x' is the same as address of saved 'x'. But this can
never happen, because heap memory and stack memory are distinct.

> With the above
> formulation this cannot happen. This error is detected a compile time -
> very convenient. Of course the following would work as well
> for(...{
> const & X x = *it; // save a const reference to the orgiinal
> ar << x
> }

Will that indeed save a const reference? How will you read const reference
from an archive?

> while the following would create an error that would go undetected.
> for(...{
> const X x = *it; // create a copy of
> ar << x
> }

Probably the right way to fix this is just don't track 'x' here. Again, no
heap allocated object will have the same address as 'x'.
>> 2. Why the "watch" command on gdb is not a reliable way to catch such
>> bugs?
> I'm not sure what the gdb watch command does but I'm sure it doesn't
> detect compile time errors.

It allows to find all places where a specific memory location is modified.
So, if you have a bug where a value of an object is modified by
serialization, it's easy to track this down to specific code line.

>>> Also, I had to tweak a number of my tests and demos to make them
>>> work with this new rule. However, the tweaking was not difficult. If
>>> altering one's code to conform this rule isn't easy, one should make
>>> an effort to understand why. Its possible that a very subtle and
>>> hard to understand bug might be being introduced.
>> Are you saying that my code is buggy?
> I'm saying you've passed up an opportunity to permit the compiler to flag
> someting that could be an error.
>>>>> ofstream ofs(av[2]);
>>>>> boost::archive::binary_oarchive oa(ofs);
>>>>> oa << *data_builder.finish();
> Looking at the above, I have to concede I never envisioned it being used
> this way. I would need more context to really comment intelligently on
> it. I will say that I alwas envisioned the interface as being used in a
> declarative style. That is, I envisioned the the serialization
> declarations
> ar & x or ar << x as a shorthand for "this member is persistent". I'm
> not saying that you're doing anything wrong, its just not what I expected.

What kind of context should I provide? This is top-level code, it reads some
files, passes them to 'data_builder' and then calls 'finish' that builds
the proper data which is then saved.

>>>>> Where 'finish' returns auto_ptr<Data>. It's looks like
>>>>> serialization checks if the serialized type is 'const' and if not,
>>>>> complains.
> correct
>>>>> Basically, it's some heuritic to prevent saving
>>>>> on-stack object
> That's one case I would like trap.

Any others?

>>>>> (though I don't understand why it would work at al).
> I don't understand this.
>>>>> I find this a bit too much . I have no reason whatsoever to make
>>>>> 'finish' return auto_ptr<const Data>. And writing
>>>>> oa << const_cast<const Data&>(*data_builder.finish());
> When finish returns something that is not a const it suggests that its
> returning a pointer to a mutable object. The serialization library
> presumes
> that objects don't change in the course of serialization. So passing a
> non-const conflicts with one of the assumptions made in the implementation
> of the library.

Ehm... the standard way to solve this is to declare "operator<<" with the
"const T&" type, just like iostreams do.

>>> That is why the STATIC_ASSERT only trips when
>>> tracking is enabled for the corresponding datatype.
>>>>> looks very strange. And modifying all places where I get this error
>>>>> is not nice too. So, can this static assert be removed?
> Its there for a purpose. How many places do you get this error? Returning
> an auto_ptr to the top of the stack is interesting to me. doesn't that
> destroy the one in the original location?

No, returning auto_ptr from a function is one of its intended usages.

> If its a temporary you could
> just
> as well return an auto_ptr to a const T. But really without more context
> its hard for me to commment.

1. I don't want to change my design due to serialization library. It's

2. Inside 'finish', I need to work with auto_ptr<T> -- because I modify
   the data. And conversion from auto_ptr<T> to auto_ptr<const T> does not
   work for me on gcc.

>> Ok,
>> 1. Why std::string is 'tracked type'?
> so that storage space isn't wasted storing repeated strings
>> 2. How do you suggest me to fix the above?
> I did suggest using & instead. But I'm curious to see more context. I'm
> curious to see more.

Here's the enclosing method:

  template<class Archive>
  void save(Archive & ar, unsigned int version) const
    // Serialization has problems with shared_ptr, so use strings.
    ar << boost::lexical_cast<std::string>(*this);

- Volodya

Boost list run by bdawes at, gregod at, cpdaniel at, john at