Use an "int status" extra parameter in property reading/writing.

When calling qt_metacall with the ReadProperty or WriteProperty, the
data is on argv[0] like it was before, but now the QVariant itself is
on argv[1] and there's an extra parameter in argv[2] which the meta
code can use to indicate result.

This allows QtDBus to process properties much more easily. In the case
of property reading, we need to be able to modify the variant itself,
because copying types when we don't have the data isn't very easy.

As for setting, we need to be able to tell setProperty to return true
or false depending on whether we succeeded in setting the property or
not.

Reviewed-By: Kent Hansen
Reviewed-By: Marius Bugge Monsen

5 files changed, 71 insertions, 32 deletions

diff --git a/src/corelib/kernel/qmetaobject.cpp b/src/corelib/kernel/qmetaobject.cpp
index 71dd5ff..34f580b 100644
--- a/src/corelib/kernel/qmetaobject.cpp
+++ b/src/corelib/kernel/qmetaobject.cpp
@@ -2131,8 +2131,15 @@ QVariant QMetaProperty::read(const QObject *object) const
             return QVariant();
         }
     }
+
+    // the status variable is changed by qt_metacall to indicate what it did
+    // this feature is currently only used by QtDBus and should not be depended
+    // upon. Don't change it without looking into QDBusAbstractInterface first
+    // -1 (unchanged): normal qt_metacall, result stored in argv[0]
+    // changed: result stored directly in value
+    int status = -1;
     QVariant value;
-    void *argv[2] = { 0, &value };
+    void *argv[] = { 0, &value, &status };
     if (t == QVariant::LastType) {
         argv[0] = &value;
     } else {
@@ -2142,8 +2149,8 @@ QVariant QMetaProperty::read(const QObject *object) const
     const_cast<QObject*>(object)->qt_metacall(QMetaObject::ReadProperty,
                                               idx + mobj->propertyOffset(),
                                               argv);
-    if (argv[1] == 0)
-        // "value" was changed
+
+    if (status != -1)
         return value;
     if (t != QVariant::LastType && argv[0] != value.data())
         // pointer or reference
@@ -2201,13 +2208,19 @@ bool QMetaProperty::write(QObject *object, const QVariant &value) const
             return false;
     }
 
-    void *argv[2] = { 0, &v };
+    // the status variable is changed by qt_metacall to indicate what it did
+    // this feature is currently only used by QtDBus and should not be depended
+    // upon. Don't change it without looking into QDBusAbstractInterface first
+    // -1 (unchanged): normal qt_metacall, result stored in argv[0]
+    // changed: result stored directly in value, return the value of status
+    int status = -1;
+    void *argv[] = { 0, &v, &status };
     if (t == QVariant::LastType)
         argv[0] = &v;
     else
         argv[0] = v.data();
     object->qt_metacall(QMetaObject::WriteProperty, idx + mobj->propertyOffset(), argv);
-    return true;
+    return status;
 }
 
 /*!
diff --git a/src/dbus/qdbusabstractinterface.cpp b/src/dbus/qdbusabstractinterface.cpp
index 08da997..9bef2dd 100644
--- a/src/dbus/qdbusabstractinterface.cpp
+++ b/src/dbus/qdbusabstractinterface.cpp
@@ -195,10 +195,10 @@ QVariant QDBusAbstractInterfacePrivate::property(const QMetaProperty &mp) const
     return QVariant();
 }
 
-void QDBusAbstractInterfacePrivate::setProperty(const QMetaProperty &mp, const QVariant &value)
+bool QDBusAbstractInterfacePrivate::setProperty(const QMetaProperty &mp, const QVariant &value)
 {
     if (!isValid || !canMakeCalls())    // can't make calls
-        return;
+        return false;
 
     // send the value
     QDBusMessage msg = QDBusMessage::createMethodCall(service, path,
@@ -208,8 +208,11 @@ void QDBusAbstractInterfacePrivate::setProperty(const QMetaProperty &mp, const Q
     msg << interface << QString::fromUtf8(mp.name()) << qVariantFromValue(QDBusVariant(value));
     QDBusMessage reply = connection.call(msg, QDBus::Block);
 
-    if (reply.type() != QDBusMessage::ReplyMessage)
+    if (reply.type() != QDBusMessage::ReplyMessage) {
         lastError = reply;
+        return false;
+    }
+    return true;
 }
 
 void QDBusAbstractInterfacePrivate::_q_serviceOwnerChanged(const QString &name,
@@ -223,6 +226,33 @@ void QDBusAbstractInterfacePrivate::_q_serviceOwnerChanged(const QString &name,
     }
 }
 
+QDBusAbstractInterfaceBase::QDBusAbstractInterfaceBase(QDBusAbstractInterfacePrivate &d, QObject *parent)
+    : QObject(d, parent)
+{
+}
+
+int QDBusAbstractInterfaceBase::qt_metacall(QMetaObject::Call _c, int _id, void **_a)
+{
+    int saved_id = _id;
+    _id = QObject::qt_metacall(_c, _id, _a);
+    if (_id < 0)
+        return _id;
+
+    if (_c == QMetaObject::ReadProperty || _c == QMetaObject::WriteProperty) {
+        QMetaProperty mp = metaObject()->property(saved_id);
+        int &status = *reinterpret_cast<int *>(_a[2]);
+        QVariant &variant = *reinterpret_cast<QVariant *>(_a[1]);
+
+        if (_c == QMetaObject::WriteProperty) {
+            status = d_func()->setProperty(mp, variant) ? 1 : 0;
+        } else {
+            variant = d_func()->property(mp);
+            status = variant.isValid() ? 1 : 0;
+        }
+        _id = -1;
+    }
+    return _id;
+}
 
 /*!
     \class QDBusAbstractInterface
@@ -247,7 +277,7 @@ void QDBusAbstractInterfacePrivate::_q_serviceOwnerChanged(const QString &name,
     This is the constructor called from QDBusInterface::QDBusInterface.
 */
 QDBusAbstractInterface::QDBusAbstractInterface(QDBusAbstractInterfacePrivate &d, QObject *parent)
-    : QObject(d, parent)
+    : QDBusAbstractInterfaceBase(d, parent)
 {
     // keep track of the service owner
     if (!d_func()->currentOwner.isEmpty())
@@ -263,7 +293,7 @@ QDBusAbstractInterface::QDBusAbstractInterface(QDBusAbstractInterfacePrivate &d,
 QDBusAbstractInterface::QDBusAbstractInterface(const QString &service, const QString &path,
                                                const char *interface, const QDBusConnection &con,
                                                QObject *parent)
-    : QObject(*new QDBusAbstractInterfacePrivate(service, path, QString::fromLatin1(interface),
+    : QDBusAbstractInterfaceBase(*new QDBusAbstractInterfacePrivate(service, path, QString::fromLatin1(interface),
                                                  con, false), parent)
 {
     // keep track of the service owner
diff --git a/src/dbus/qdbusabstractinterface.h b/src/dbus/qdbusabstractinterface.h
index 6400b26..e525f77 100644
--- a/src/dbus/qdbusabstractinterface.h
+++ b/src/dbus/qdbusabstractinterface.h
@@ -61,7 +61,23 @@ class QDBusError;
 class QDBusPendingCall;
 
 class QDBusAbstractInterfacePrivate;
-class QDBUS_EXPORT QDBusAbstractInterface: public QObject
+
+class QDBUS_EXPORT QDBusAbstractInterfaceBase: public QObject
+{
+public:
+    int qt_metacall(QMetaObject::Call, int, void**);
+protected:
+    QDBusAbstractInterfaceBase(QDBusAbstractInterfacePrivate &dd, QObject *parent);
+private:
+    Q_DECLARE_PRIVATE(QDBusAbstractInterface)
+};
+
+class QDBUS_EXPORT QDBusAbstractInterface:
+#ifdef Q_QDOC
+        public QObject
+#else
+        public QDBusAbstractInterfaceBase
+#endif
 {
     Q_OBJECT
 
diff --git a/src/dbus/qdbusabstractinterface_p.h b/src/dbus/qdbusabstractinterface_p.h
index e5822b3..e577898 100644
--- a/src/dbus/qdbusabstractinterface_p.h
+++ b/src/dbus/qdbusabstractinterface_p.h
@@ -87,7 +87,7 @@ public:
 
     // these functions do not check if the property is valid
     QVariant property(const QMetaProperty &mp) const;
-    void setProperty(const QMetaProperty &mp, const QVariant &value);
+    bool setProperty(const QMetaProperty &mp, const QVariant &value);
 
     // return conn's d pointer
     inline QDBusConnectionPrivate *connectionPrivate() const
diff --git a/src/dbus/qdbusinterface.cpp b/src/dbus/qdbusinterface.cpp
index 211b717..6f61847 100644
--- a/src/dbus/qdbusinterface.cpp
+++ b/src/dbus/qdbusinterface.cpp
@@ -205,26 +205,6 @@ int QDBusInterfacePrivate::metacall(QMetaObject::Call c, int id, void **argv)
             // done
             return -1;
         }
-    } else if (c == QMetaObject::ReadProperty) {
-        // Qt doesn't support non-readable properties
-        // we have to re-check
-        QMetaProperty mp = metaObject->property(id + metaObject->propertyOffset());
-        if (!mp.isReadable())
-            return -1;          // don't read
-
-        QVariant *value = reinterpret_cast<QVariant*>(argv[1]);
-        argv[1] = 0;
-        *value = property(mp);
-
-        return -1;              // handled, error or not
-    } else if (c == QMetaObject::WriteProperty) {
-        // QMetaProperty::write has already checked that we're writable
-        // it has also checked that the type is right
-        QVariant *value = reinterpret_cast<QVariant *>(argv[1]);
-        QMetaProperty mp = metaObject->property(id + metaObject->propertyOffset());
-
-        setProperty(mp, *value);
-        return -1;
     }
     return id;
 }