[Bug 43222] Add Qt5 support

bugzilla-daemon at freedesktop.org bugzilla-daemon at freedesktop.org
Thu Nov 24 17:01:29 CET 2011


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

Olli Salli <ollisal at gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ollisal at gmail.com

--- Comment #3 from Olli Salli <ollisal at gmail.com> 2011-11-24 08:01:29 PST ---
-# 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.

+Requires.private: QtCore >= ${QT_MIN_VERSION}, QtDBus >= ${QT_MIN_VERSION},
QtNetwork >= ${QT_MIN_VERSION}, QtXml >= ${QT_MIN_VERSION}

Should be max version too, as they use the same name. So it doesn't find qt5
when you're linking against qt4 build of tp-qt.

Ahem that also means we should also cater for parallel installing both a
TelepathyQt4 and a TelepathyQt5 cmake file, just like the lib binary.

    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.

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.

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



More information about the telepathy-bugs mailing list