config: add replies and dbus api documentation

Add replies, which use standard X error values, to the two currently-supported
input configuration requests.
Document the D-BUS API we use.
Make sure we free everything when we encounter an error.
Add a _source option to all incoming requests, noting that it came from a
client.
Reject all requests to add a device where an option name contains an
underscore.

2 files changed, 214 insertions, 113 deletions

diff --git a/config/config.c b/config/config.c
index 8cb4406b5..e3c33aaa1 100644
--- a/config/config.c
+++ b/config/config.c
@@ -29,17 +29,26 @@
 #define DBUS_API_SUBJECT_TO_CHANGE
 #include <dbus/dbus.h>
 #include <string.h>
+
+#include <X11/X.h>
+
 #include "opaque.h" /* for 'display': there has to be a better way */
                     /* the above comment lies.  there is no better way. */
 #include "input.h"
+#include "inputstr.h"
 #include "config.h"
+#include "os.h"
 
 #define MATCH_RULE "type='method_call',interface='org.x.config.input'"
 
-#define MALFORMED_MSG "config: malformed message, dropping"
-#define MALFORMED_MESSAGE() DebugF(MALFORMED_MSG)
-#define MALFORMED_MESSAGE_ERROR() DebugF(MALFORMED_MSG ": %s, %s", \
-                                       error.name, error.message)
+#define MALFORMED_MSG "[config] malformed message, dropping"
+#define MALFORMED_MESSAGE() { DebugF(MALFORMED_MSG "\n"); \
+                            ret = BadValue; \
+                            goto unwind; }
+#define MALFORMED_MESSAGE_ERROR() { DebugF(MALFORMED_MSG ": %s, %s", \
+                                       error->name, error->message); \
+                                  ret = BadValue; \
+                                  goto unwind; }
 
 static DBusConnection *configConnection = NULL;
 static int configfd = -1;
@@ -55,130 +64,186 @@ configDispatch()
     dbus_connection_read_write_dispatch(configConnection, 0);
 }
 
-static DBusHandlerResult
-configMessage(DBusConnection *connection, DBusMessage *message, void *closure)
+static int
+configAddDevice(DBusMessage *message, DBusMessageIter *iter, DBusError *error)
 {
-    InputOption *option = NULL, *ret = NULL;
-    DBusMessageIter iter, subiter;
-    DBusError error;
+    DBusMessageIter subiter;
+    InputOption *tmpo = NULL, *options = NULL;
     char *tmp = NULL;
+    int ret = BadMatch;
+
+    DebugF("[config] adding device\n");
+
+    /* signature should be [ss][ss]... */
+    options = (InputOption *) xcalloc(sizeof(InputOption), 1);
+    if (!options) {
+        ErrorF("[config] couldn't allocate option\n");
+        return BadAlloc;
+    }
+
+    options->key = xstrdup("_source");
+    options->value = xstrdup("client/dbus");
+
+    while (dbus_message_iter_get_arg_type(iter) == DBUS_TYPE_ARRAY) {
+        tmpo = (InputOption *) xcalloc(sizeof(InputOption), 1);
+        if (!tmpo) {
+            ErrorF("[config] couldn't allocate option\n");
+            ret = BadAlloc;
+            goto unwind;
+        }
+
+        dbus_message_iter_recurse(iter, &subiter);
+
+        if (dbus_message_iter_get_arg_type(&subiter) != DBUS_TYPE_STRING)
+            MALFORMED_MESSAGE();
+
+        dbus_message_iter_get_basic(&subiter, &tmp);
+        if (!tmp)
+            MALFORMED_MESSAGE();
+        if (tmp[0] == '_') {
+            ErrorF("[config] attempted subterfuge: option name %s given\n",
+                   tmp);
+            MALFORMED_MESSAGE();
+        }
+        tmpo->key = xstrdup(tmp);
+        if (!tmpo->key) {
+            ErrorF("[config] couldn't duplicate key!\n");
+            ret = BadAlloc;
+            goto unwind;
+        }
+
+        if (!dbus_message_iter_has_next(&subiter))
+            MALFORMED_MESSAGE();
+        dbus_message_iter_next(&subiter);
+        if (dbus_message_iter_get_arg_type(&subiter) != DBUS_TYPE_STRING)
+            MALFORMED_MESSAGE();
+
+        dbus_message_iter_get_basic(&subiter, &tmp);
+        if (!tmp)
+            MALFORMED_MESSAGE();
+        tmpo->value = xstrdup(tmp);
+        if (!tmpo->value) {
+            ErrorF("[config] couldn't duplicate option!\n");
+            ret = BadAlloc;
+            goto unwind;
+        }
+
+        tmpo->next = options;
+        options = tmpo;
+        dbus_message_iter_next(iter);
+    }
+
+    ret = NewInputDeviceRequest(options);
+    if (ret != Success)
+        DebugF("[config] NewInputDeviceRequest failed\n");
+
+    return ret;
+
+unwind:
+    if (tmpo->key)
+        xfree(tmpo->key);
+    if (tmpo->value)
+        xfree(tmpo->value);
+    if (tmpo)
+        xfree(tmpo);
+
+    while (options) {
+        tmpo = options;
+        options = options->next;
+        if (tmpo->key)
+            xfree(tmpo->key);
+        if (tmpo->value)
+            xfree(tmpo->value);
+        xfree(tmpo);
+    }
+
+    return ret;
+}
+
+static int
+configRemoveDevice(DBusMessage *message, DBusMessageIter *iter,
+                   DBusError *error)
+{
     int deviceid = -1;
+    int ret = BadMatch;
     DeviceIntPtr pDev = NULL;
 
+    if (!dbus_message_get_args(message, error, DBUS_TYPE_INT32,
+                               &deviceid, DBUS_TYPE_INVALID)) {
+        MALFORMED_MESSAGE_ERROR();
+    }
+
+    if (deviceid < 0 || !(pDev = LookupDeviceIntRec(deviceid))) {
+        DebugF("[config] bogus device id %d given\n", deviceid);
+        ret = BadMatch;
+        goto unwind;
+    }
+
+    DebugF("[config] removing device %s (id %d)\n", pDev->name, deviceid);
+
+    /* Call PIE here so we don't try to dereference a device that's
+     * already been removed. */
+    OsBlockSignals();
+    ProcessInputEvents();
+    RemoveDevice(pDev);
+    OsReleaseSignals();
+
+    return Success;
+
+unwind:
+    return ret;
+}
+
+static DBusHandlerResult
+configMessage(DBusConnection *connection, DBusMessage *message, void *closure)
+{
+    DBusMessageIter iter;
+    DBusError error;
+    DBusMessage *reply;
+    DBusConnection *bus = closure;
+    int ret = BadDrawable; /* nonsensical value */
+
     dbus_error_init(&error);
 
     if (strcmp(dbus_message_get_interface(message),
                "org.x.config.input") == 0) {
         if (!dbus_message_iter_init(message, &iter)) {
-            ErrorF("config: failed to init iterator\n");
+            ErrorF("[config] failed to init iterator\n");
             dbus_error_free(&error);
             return DBUS_HANDLER_RESULT_NEED_MEMORY; /* ?? */
         }
-        if (strcmp(dbus_message_get_member(message), "add") == 0) {
-            DebugF("config: adding device\n");
-            /* signature should be [ss][ss]... */
-            while (dbus_message_iter_get_arg_type(&iter) == DBUS_TYPE_ARRAY) {
-                option = (InputOption *)xcalloc(sizeof(InputOption), 1);
-                if (!option) {
-                    while (ret) {
-                        option = ret;
-                        ret = ret->next;
-                        xfree(option);
-                    }
-                    dbus_error_free(&error);
-                    return DBUS_HANDLER_RESULT_NEED_MEMORY;
-                }
-
-                dbus_message_iter_recurse(&iter, &subiter);
-
-                if (dbus_message_iter_get_arg_type(&subiter) !=
-                    DBUS_TYPE_STRING) {
-                    MALFORMED_MESSAGE();
-                    xfree(option);
-                    dbus_error_free(&error);
-                    return DBUS_HANDLER_RESULT_HANDLED;
-                }
-                dbus_message_iter_get_basic(&subiter, &tmp);
-                if (!tmp) {
-                    MALFORMED_MESSAGE();
-                    xfree(option);
-                    break;
-                }
-                option->key = xstrdup(tmp);
-                if (!option->key) {
-                    ErrorF("couldn't duplicate the key!\n");
-                    xfree(option);
-                    break;
-                }
-
-                if (!dbus_message_iter_has_next(&subiter)) {
-                    MALFORMED_MESSAGE();
-                    xfree(option->key);
-                    xfree(option);
-                    dbus_error_free(&error);
-                    return DBUS_HANDLER_RESULT_HANDLED;
-                }
-                dbus_message_iter_next(&subiter);
-
-                if (dbus_message_iter_get_arg_type(&subiter) !=
-                    DBUS_TYPE_STRING) {
-                    MALFORMED_MESSAGE();
-                    xfree(option);
-                    dbus_error_free(&error);
-                    return DBUS_HANDLER_RESULT_HANDLED;
-                }
-                dbus_message_iter_get_basic(&subiter, &tmp);
-                if (!tmp) {
-                    MALFORMED_MESSAGE();
-                    xfree(option->key);
-                    xfree(option);
-                    break;
-                }
-                option->value = xstrdup(tmp);
-                if (!option->value) {
-                    ErrorF("couldn't duplicate the option!\n");
-                    xfree(option->value);
-                    xfree(option);
-                    break;
-                }
-
-                option->next = ret;
-                ret = option;
-                dbus_message_iter_next(&iter);
-            }
-
-            if (NewInputDeviceRequest(ret) != Success) {
-                DebugF("config: NewInputDeviceRequest failed\n");
-            }
-            dbus_error_free(&error);
-            return DBUS_HANDLER_RESULT_HANDLED;
-        }
-        else if (strcmp(dbus_message_get_member(message), "remove") == 0) {
-            ErrorF("config: removing device\n");
-            if (!dbus_message_get_args(message, &error, DBUS_TYPE_INT32,
-                                       &deviceid, DBUS_TYPE_INVALID)) {
-                MALFORMED_MESSAGE_ERROR();
-                dbus_error_free(&error);
-                return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
-            }
-            if (deviceid < 0 || !(pDev = LookupDeviceIntRec(deviceid))) {
-                DebugF("config: bogus device id %d given\n", deviceid);
-                dbus_error_free(&error);
-                return DBUS_HANDLER_RESULT_HANDLED;
-            }
-            /* Call PIE here so we don't try to dereference a device that's
-             * already been removed.  Technically there's still a small race
-             * here, so we should ensure that SIGIO is blocked. */
-            ProcessInputEvents();
-            RemoveDevice(pDev);
+
+        if (strcmp(dbus_message_get_member(message), "add") == 0)
+            ret = configAddDevice(message, &iter, &error);
+        else if (strcmp(dbus_message_get_member(message), "remove") == 0)
+            ret = configRemoveDevice(message, &iter, &error);
+    }
+
+    if (ret != BadDrawable && ret != BadAlloc) {
+        reply = dbus_message_new_method_return(message);
+        dbus_message_iter_init_append(reply, &iter);
+
+        if (!dbus_message_iter_append_basic(&iter, DBUS_TYPE_INT32, &ret)) {
+            ErrorF("[config] couldn't append to iterator\n");
             dbus_error_free(&error);
             return DBUS_HANDLER_RESULT_HANDLED;
         }
+
+        if (!dbus_connection_send(bus, reply, NULL))
+            ErrorF("[config] failed to send reply\n");
+        dbus_connection_flush(bus);
+
+        dbus_message_unref(reply);
     }
 
     dbus_error_free(&error);
 
-    return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
+    if (ret == BadAlloc)
+        return DBUS_HANDLER_RESULT_NEED_MEMORY;
+    else if (ret == BadDrawable)
+        return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
+    else
+        return DBUS_HANDLER_RESULT_HANDLED;
 }
 
 void
@@ -231,7 +296,7 @@ configInitialise()
 
     vtable.message_function = configMessage;
     snprintf(busobject, sizeof(busobject), "/org/x/config/%d", atoi(display));
-    if (!dbus_connection_register_object_path(bus, busobject, &vtable, NULL)) {
+    if (!dbus_connection_register_object_path(bus, busobject, &vtable, bus)) {
         configfd = -1;
         dbus_bus_release_name(bus, busname, &error);
         dbus_bus_remove_match(bus, MATCH_RULE, &error);
@@ -243,9 +308,7 @@ configInitialise()
     DebugF("[dbus] registered object path %s\n", busobject);
 
     dbus_error_free(&error);
-
     configConnection = bus;
-
     AddGeneralSocket(configfd);
 }
 
@@ -265,7 +328,9 @@ configFini()
         dbus_error_free(&error);
     }
 }
-#else
+
+#else /* !HAVE_DBUS */
+
 void
 configDispatch()
 {
@@ -280,4 +345,5 @@ void
 configFini()
 {
 }
-#endif
+
+#endif /* HAVE_DBUS */
diff --git a/config/dbus-api b/config/dbus-api
new file mode 100644
index 000000000..53bb3e45d
--- /dev/null
+++ b/config/dbus-api
@@ -0,0 +1,35 @@
+D-BUS Configuration API v0.1
+----------------------------
+
+The X server will register the bus name org.x.config.displayN, and the 
+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.add:
+        Takes an argument of key/value option pairs in arrays, e.g.:
+         [ss][ss][ss][ss]
+        is the signature for four options.  These options will be passed
+        to the input driver as with any others.
+        Option names beginning with _ are not allowed; they are reserved
+        for internal use.
+
+        Returns one int32, which is an X Status, as defined in X.h.  If
+        everything is successful, Success will be returned.  BadMatch will
+        be returned if the options given do not match any device.  BadValue
+        is returned for a malformed message.
+
+        Notably, BadAlloc is never returned: the server internally signals
+        to D-BUS that the attempt failed for lack of memory.
+
+        The return does not notify the client of which devices were created
+        or modified as a result of this request: clients are encouraged to
+        listen for the XInput DevicePresenceNotify event to monitor changes
+        in the device list.
+
+    org.x.config.input.remove:
+        Takes one int32 argument, which is the device ID to remove, i.e.:
+         i
+        is the signature.
+        Same return values as org.x.config.input.add.