[Bug 43222] Add Qt5 support

bugzilla-daemon at freedesktop.org bugzilla-daemon at freedesktop.org
Thu Nov 24 22:45:57 CET 2011


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

--- Comment #6 from Andre Moreira Magalhaes <andrunko at gmail.com> 2011-11-24 13:45:57 PST ---
(In reply to comment #3)
> -# Find qt4 version >= 4.5
> -set (QT_MIN_VERSION "4.6.0")
> -find_package(Qt4 REQUIRED)
> +# Find qt4 version >= 4.8 or qt5 >= 5.0.0
> +set(QT4_MIN_VERSION "4.8.0")
> 
> No reason to bump qt4 dep to 4.8.
Fixed

>     explicit inline SharedPtr(const QWeakPointer<T> &o)
>     {
>         if (o.data() && o.data()->strongref.fetchAndAddOrdered(0) > 0) {
>             d = static_cast<T*>(o.data());
>             d->ref();
>         }
> 
> Used like that, the atomic integer is completely useless. First after the
> o.data() check, an another thread can cause it to go NULL, and then after the
> refcount check, an another thread can cause it to drop to zero and the object
> be deleted.
> 
> This is perhaps also why they removed the implicit integer conversion in qt5 -
> the whole point of the atomic integer is to be able to implement
> inspection/test AND modify in one atomic operation to guard against an another
> thread ruining our day in between them. And that removal of API helped us to
> notice the pathological usage here.
> 
> If the answer is "even this part of tp-qt4 doesn't need to be thread-safe" then
> why are we even using an atomic integer? If so, it serves no purpose other than
> to slow things down even in single-threaded usage... However passing object
> refs across threads safely (even if not using the objects pointed to from
> multiple threads at a time) seems useful so it might be wise to clone this into
> an another bug, to be resolved before committing this inline header to a 0.9.0
> release.
> 
> I suggest taking a look at other Qt shared pointer impls (webkit qt's ?
> qsharedpointers? qexplicitlyshared...'s ?) for ideas on getting the thread
> safety right.
This is true, could you please file a bug for that with the rationale above. In
the meantime I will leave this branch using fetchAndAddOrdered, so it works
with both qt4 and qt5

> channel-class-spec.h:
> 
>     // TODO: Use new TP_QT_... constants
>     QString channelType() const
>     {
>         return qdbus_cast<QString>(
>                 property(TP_QT_IFACE_CHANNEL + QLatin1String(".ChannelType")));
> It seems we do now. In general, please check the TODOs for stuff you might have
> already done.
This is bug #43223

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



More information about the telepathy-bugs mailing list