Hotplug: D-Bus: API version 2

Use uint32s instead of int32s where practical, and add an API version
request.  Also, try to return all devices added, not just the first,
and box device arguments.

2 files changed, 53 insertions, 18 deletions

diff --git a/config/dbus-api b/config/dbus-api
index 654c22bec..018e98657 100644
--- a/config/dbus-api
+++ b/config/dbus-api
@@ -1,4 +1,4 @@
-D-BUS Configuration API v0.1
+D-BUS Configuration API v2
 ----------------------------
 
 The X server will register the bus name org.x.config.displayN, and the 
@@ -7,6 +7,9 @@ object /org/x/config/N, where N is the display number.
 Currently only hotplugging of input devices is supported.
 
 org.x.config.input:
+    org.x.config.input.version:
+        Returns one unsigned int32, which is the API version.
+
     org.x.config.input.add:
         Takes an argument of key/value option pairs in arrays, e.g.:
          [ss][ss][ss][ss]
@@ -15,18 +18,18 @@ org.x.config.input:
         Option names beginning with _ are not allowed; they are reserved
         for internal use.
 
-        Returns one signed int32, which is the device id of the new device.
-        If the return value is a negative number, it represents the X
-        Status, as defined in X.h. BadMatch will be returned if the options
+        Returns a number of signed int32s.  Positive integers are the
+        device IDs of new devices; negative numbers are X error codes,
+        as defined in X.h.  BadMatch will be returned if the options
         given do not match any device.  BadValue is returned for a malformed
-        message.  (Example: 8 is new device id 8. -8 is BadMatch.)
+        message.  (Example: 8 is new device ID 8; -8 is BadMatch.)
 
         Notably, BadAlloc is never returned: the server internally signals
         to D-BUS that the attempt failed for lack of memory.
 
     org.x.config.input.remove:
-        Takes one int32 argument, which is the device ID to remove, i.e.:
-         i
+        Takes one uint32 argument, which is the device ID to remove, i.e.:
+         u
         is the signature.
 
         Returns one signed int32 which represents an X status as defined in
@@ -34,4 +37,4 @@ org.x.config.input:
 
     org.x.config.input.listDevices:
         Lists the currently active devices. No argument. 
-        Return value is sequence of <id> <name> <id> <name> ...
+        Return value is sequence of [<id> <name>] [<id> <name>] ..., i.e. [us].
diff --git a/config/dbus.c b/config/dbus.c
index 2450e0c00..861aa8ee8 100644
--- a/config/dbus.c
+++ b/config/dbus.c
@@ -36,7 +36,7 @@
 #include "input.h"
 #include "inputstr.h"
 
-#define API_VERSION 1
+#define API_VERSION 2
 
 #define MATCH_RULE "type='method_call',interface='org.x.config.input'"
 
@@ -156,11 +156,17 @@ add_device(DBusMessage *message, DBusMessage *reply, DBusError *error)
         goto unwind;
     }
 
-    if (!dbus_message_iter_append_basic(&reply_iter, DBUS_TYPE_INT32,
-                                        &dev->id)) {
-        ErrorF("[config/dbus] couldn't append to iterator\n");
-        ret = BadAlloc;
-        goto unwind;
+    /* XXX: If we fail halfway through, we don't seem to have any way to
+     *      empty the iterator, so you'll end up with some device IDs,
+     *      plus an error.  This seems to be a shortcoming in the D-Bus
+     *      API. */
+    for (; dev; dev = dev->next) {
+        if (!dbus_message_iter_append_basic(&reply_iter, DBUS_TYPE_INT32,
+                                            &dev->id)) {
+            ErrorF("[config/dbus] couldn't append to iterator\n");
+            ret = BadAlloc;
+            goto unwind;
+        }
     }
 
 unwind:
@@ -198,7 +204,7 @@ remove_device(DBusMessage *message, DBusMessage *reply, DBusError *error)
     }
     dbus_message_iter_init_append(reply, &reply_iter);
 
-    if (!dbus_message_get_args(message, error, DBUS_TYPE_INT32,
+    if (!dbus_message_get_args(message, error, DBUS_TYPE_UINT32,
                                &deviceid, DBUS_TYPE_INVALID)) {
         MALFORMED_MESSAGE_ERROR();
     }
@@ -232,21 +238,45 @@ static int
 list_devices(DBusMessage *message, DBusMessage *reply, DBusError *error)
 {
     DeviceIntPtr dev;
-    DBusMessageIter iter;
+    DBusMessageIter iter, subiter;
 
     dbus_message_iter_init_append(reply, &iter);
 
     for (dev = inputInfo.devices; dev; dev = dev->next) {
-        if (!dbus_message_iter_append_basic(&iter, DBUS_TYPE_INT32,
+        if (!dbus_message_iter_open_container(&iter, DBUS_TYPE_STRUCT, NULL,
+                                              &subiter)) {
+            ErrorF("[config/dbus] couldn't init container\n");
+            return BadAlloc;
+        }
+        if (!dbus_message_iter_append_basic(&subiter, DBUS_TYPE_UINT32,
                                             &dev->id)) {
             ErrorF("[config/dbus] couldn't append to iterator\n");
             return BadAlloc;
         }
-        if (!dbus_message_iter_append_basic(&iter, DBUS_TYPE_STRING,
+        if (!dbus_message_iter_append_basic(&subiter, DBUS_TYPE_STRING,
                                             &dev->name)) {
             ErrorF("[config/dbus] couldn't append to iterator\n");
             return BadAlloc;
         }
+        if (!dbus_message_iter_close_container(&iter, &subiter)) {
+            ErrorF("[config/dbus] couldn't close container\n");
+            return BadAlloc;
+        }
+    }
+
+    return Success;
+}
+
+static int
+get_version(DBusMessage *message, DBusMessage *reply, DBusError *error)
+{
+    DBusMessageIter iter;
+    unsigned int version = API_VERSION;
+
+    dbus_message_iter_init_append(reply, &iter);
+    if (!dbus_message_iter_append_basic(&iter, DBUS_TYPE_UINT32, &version)) {
+        ErrorF("[config/dbus] couldn't append version\n");
+        return BadAlloc;
     }
 
     return Success;
@@ -282,6 +312,8 @@ message_handler(DBusConnection *connection, DBusMessage *message, void *data)
         err = remove_device(message, reply, &error);
     else if (strcmp(dbus_message_get_member(message), "listDevices") == 0)
         err = list_devices(message, reply, &error);
+    else if (strcmp(dbus_message_get_member(message), "version") == 0)
+        err = get_version(message, reply, &error);
     else
         goto err_reply;