|
Boost : |
From: Greg Colvin (gcolvin_at_[hidden])
Date: 2001-09-02 09:48:05
From: Peter Dimov <pdimov_at_[hidden]>
To: <boost_at_[hidden]>
Sent: Sunday, September 02, 2001 4:41 AM
Subject: Re: [boost] shared_xxx issues
> From: "Greg Colvin" <gcolvin_at_[hidden]>
> > From: David Abrahams <david.abrahams_at_[hidden]>
>
> [...]
>
> > > > > Second Issue:
> > > > >
> > > > > I found the following code in shared_array (and I assume the analogous code
> > > > > is in shared_ptr):
> > > > >
> > > > > void reset(T* p=0)
> > > > > {
> > > > > if ( px == p ) return; // fix: self-assignment safe
> > > > >
> > > > > What this does is to make a silent success of resetting a shared_ptr with
> > > > > the pointer it owns. The problem with that is that resetting a shared_ptr to
> > > > > any /other/ pointer which is managed by a shared_ptr is a fatal (and
> > > > > hard-to-detect) error, which happens even to experiencedprogrammers. I
> > > > > don't think the trade-off is a good one. Making this special case (which we
> > > > > can detect) silently work when all other cases will fail without complaint
> > > > > is a mistake. I'd rather see an assert() here.
>
> [...]
>
> > You've almost convinced me. What is bugging me is a desire to
> > stay consistent with auto_ptr:
> >
> > void reset(X* p=0) throw();
> > Effects:
> > If get() != p then delete get().
> > Postconditions:
> > *this holds the pointer p.
>
> I think that Dave is right. I don't know why this made it into
> auto_ptr::reset, but the above shared_ptr::reset does not conform to its
> definition:
Right. Either code the is wrong, or the definition, or both.
I don't recall who made the "fix" for self-reset, but the effect
is to make self-reset consistent with self-assignment, as it is
for auto_ptr. I'm back to not being convinced that this is a
problem, and am inclined to fix the definition.
Just because it is possible to stumble into undefined behavior
in many ways doesn't mean we shouldn't prevent this one way.
> "shared_ptr reset
>
> void reset( T* p=0 );
> First, if use_count() == 1, deletes the object pointed to by the stored
> pointer. Otherwise, use_count() for any remaining copies is decremented by
> 1.
>
> Then replaces the contents of this, as if by storing a copy of p, which must
> have been allocated via a C++ new expression or be 0. Afterwards,
> use_count() is 1 (even if p==0; see ~shared_ptr). Note that in C++ delete
> on a pointer with a value of 0 is harmless.
>
> The only exception which may be thrown is std::bad_alloc. If an exception
> is thrown, delete p is called."
>
> The 'fix' makes the reset() a no-op, and this is not allowed by the
> specification. Note the effect 'decrements use_count for remaining copies'
> and the postcondition 'use_count() == 1'. (BTW an assert is not allowed
> either by this definition. The undefined behavior would happen at
> destruction, not at reset.)
>
> That's why in my implementation (after staring at the current
> shared_ptr::reset() code for a while) I simply used
>
> void reset(T * p = 0)
> {
> shared_ptr(p).swap(*this);
> }
Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk