[Bug 43239] Tp::SharedPtr tries to be thread-safe, but secretly isn't

[bugzilla-daemon at freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=43239

Andre Moreira Magalhaes <andrunko at gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|                            |FIXED

--- Comment #9 from Andre Moreira Magalhaes <andrunko at gmail.com> 2011-12-06 08:23:52 PST ---
(In reply to comment #8)
> (In reply to comment #7)
> > (In reply to comment #5)
> > > 1) make WeakPtr constructible from raw pointer, if there isn't some issue I'm
> > > overlooking, and use it wherever sensible
> > Added
> 
> Yeah looks good now.
> 
> > 
> > > 2) explain in the docs, NEWS API changes section and commit message why
> > > promotion from QWeakPointer is gone
> > Done in the docs (NEWS will be updated when merging), but I am not really happy
> > with this, suggestions are welcome.
> 
> Otherwise it's ~fine but this bit
> 
> + * \brief The WeakPtr class holds a weak reference to a shared pointer.
> 
> is a bit weird. The reference is to a "object managed by SharedPtrs", not to a
> shared pointer (the SharedPtr).
> 
> You could also mention that it's useful for breaking reference cycles which
> would result from using a SharedPtr for both ends of a pair of mutually linked
> objects to refer to each other.
Done

> > 
> > > 3) ponder making SharedCount more of an internal implementation detail
> > > (compiler checkably) than a public class, because we *can* if my memory serves
> > > me correctly on the template <class X...> friend class Y; syntax
> > Done
> > 
> 
> Mm, couldn't you further move the entire declaration of class SharedCount here
> 
> > class TP_QT_EXPORT RefCounted
> > {
> >     Q_DISABLE_COPY(RefCounted)
> > 
> > public:
> 
> Before the start of the public section? This would hide the class completely
> from external (unintended) usage. (Needs changing the three lines which refer
> to SharedCount as global in WeakPtr and SharedPtr of course in addition, but
> nothing else afaics?). Then the header would only declare RefCounted, WeakPtr
> and SharedPtr as public types, which is what we want.
Done

> With these, merge please.
Thanks for the review, changes merged upstream. It will be in tp-qt 0.9.0

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA Contact for the bug.