On 09/02/2008, Curt Arnold <[EMAIL PROTECTED]> wrote:
>
> Had a recent issue with const on accessors: 
> https://issues.apache.org/jira/browse/LOGCXX-202

That looks unrelated (except for being on the same line of code).
In the context of those const member functions the type of p is "const
pointer to non-const T" so there is no cast needed to return a
pointer/reference to non-const T.
Constness is shallow in this respect (and since smart pointers model
builtin pointer semantics, shallow constness is what you want - with
e.g. containers it's not.)

> I did play a bit a few weeks ago trying to make things more correct,
> but I started have things start unraveling and I decided spending a
> substantial amount of time trying to repair code that hasn't had any
> reported issues was not the best use of my time.

I spotted a few other non-const-correct areas, I'll try to review them tomorrow.

> The operator< is required for DLL exporting STL collections of
> ObjectPtr's with some version of Microsoft VC (believe it was VC6).
> It should never be called in practice, but is required to be present
> to successfully link.

OK, then it shouldn't matter (comparing pointers with < works on all
common platforms, it's just not guaranteed to on all architectures)

> > The attached patch addresses all these points, could it be considered
> > for inclusion?
>
> I'd definitely separate these concerns so they could be independently
> rolled back if there were unforeseen complications.  Would be great if
> you could file bugs for each of the concerns (http://issues.apache.org/jira
> ).  Something like
>
> LOGCXX-nnn: Unnecessary casts in ObjectPtrT
> LOGCXX-nnn:  Add ObjectPtrT::exchange
> ...

Sure, I'll do that now.

> > It seems from its implementation that ObjectPtrBase::exchange() is not
> > atomic on 64-bit UNIX platforms, are there any plans to change that?
> >
>
>
> APR introduced a apr_xchgptr function at my request, however I don't
> think it has made it into a released version of APR.  If you want to
> propose changes to configure.in that could detect if the APR version
> had apr_xchgptr, I'd be happy to have that as a conditional in the
> implementation of ObjectPtrBase::exchange.

Thanks for the pointer - I'll look into it.

Jonathan

Reply via email to