[RFC PATCH] Handle hotplugged devices from xorg.conf

Dan Nicholson dbn.lists at gmail.com
Wed Dec 3 07:20:26 PST 2008


Hotplug hooks are added to the HAL core to allow the DDX to handle device
hotplugging. If a registered hook returns False, config/hal will continue
processing the event. This allows the DDX to examine the type of device
and deciding whether to handle it or not.

For xorg, two hooks have been added: xf86HotplugAddDevice and
xf86HotplugRemoveDevice. Given a path to the device file from config/hal,
xorg.conf is searched for an InputDevice with matching Device or Path.
When AllowEmptyInput is false, core devices are skipped since they're
picked up in HandleConfigFile.

The config_info field is marked with "xf86:" followed by the udi from
HAL. This allows the device to be identified on remove and distinguishes
it from devices that have been handled by config/hal. This is also used
in DeleteInputDeviceRequest since hotplugged xorg.conf devices will not
be in the active layout, but cannot have their data.

Signed-off-by: Dan Nicholson <dbn.lists at gmail.com>
---
 This seems to work well in my testing, although I'm not sure about
 leaking on device removal. Crash on replug forced me to change the test
 in DeleteInputDeviceRequest for HAL devices.

 Issues:
 * config_hal_hotplug_hook should probably be a linked list, but this is
   all I needed for now.
 * I think there's race with HandleConfigFile. config_init happens early
   in main and HandleConfigFile doesn't happen until later in InitOutput.
   I didn't hit a problem with this in my testing, but I don't know what
   would prevent it from happening. One thing I thought of is just to busy
   wait in xf86HotplugAddDevice until xf86configptr becomes non-NULL.
 * I'm not sure the logic with allowEmptyInput and autoEnableDevices is
   correct. Peter?

 config/hal.c                   |   31 ++++++
 hw/xfree86/common/xf86Config.c |  221 ++++++++++++++++++++++++++++++++++++++++
 hw/xfree86/common/xf86Config.h |    3 +
 hw/xfree86/common/xf86Init.c   |   20 ++++
 hw/xfree86/common/xf86Xinput.c |   12 +-
 include/hotplug.h              |   14 +++
 6 files changed, 295 insertions(+), 6 deletions(-)

diff --git a/config/hal.c b/config/hal.c
index 8dfbb07..d9b51e4 100644
--- a/config/hal.c
+++ b/config/hal.c
@@ -58,6 +58,8 @@ struct xkb_options {
     char* options;
 };
 
+/* Allow the ddx to handle hotplugged devices. */
+static struct config_hal_hotplug_hook *hotplug_hook;
 
 static void
 remove_device(DeviceIntPtr dev)
@@ -79,6 +81,13 @@ device_removed(LibHalContext *ctx, const char *udi)
     DeviceIntPtr dev, next;
     char *value;
 
+    /* See if the ddx wants to handle this */
+    if (hotplug_hook && hotplug_hook->remove && hotplug_hook->remove(udi)) {
+        LogMessageVerb(X_INFO, 4,
+                       "config/hal: device removed in hook. Ignoring.\n");
+        return;
+    }
+
     value = xalloc(strlen(udi) + 5); /* "hal:" + NULL */
     if (!value)
         return;
@@ -255,6 +264,14 @@ device_added(LibHalContext *hal_ctx, const char *udi)
         goto unwind;
     }
 
+    /* See if the ddx wants to handle this */
+    if (hotplug_hook && hotplug_hook->add && hotplug_hook->add(path, udi)) {
+        LogMessageVerb(X_INFO, 4,
+                       "config/hal: device %s handled in hook. Ignoring.\n",
+                       name);
+        goto unwind;
+    }
+
     /* ok, grab options from hal.. iterate through all properties
     * and lets see if any of them are options that we can add */
     set = libhal_device_get_all_properties(hal_ctx, udi, &error);
@@ -529,6 +546,18 @@ out_err:
     return;
 }
 
+void
+config_hal_hotplug_set_hook(struct config_hal_hotplug_hook *hook)
+{
+    hotplug_hook = hook;
+}
+
+void
+config_hal_hotplug_clear_hook(void)
+{
+    hotplug_hook = NULL;
+}
+
 static struct config_hal_info hal_info;
 static struct config_dbus_core_hook hook = {
     .connect = connect_hook,
@@ -543,6 +572,8 @@ config_hal_init(void)
     hal_info.system_bus = NULL;
     hal_info.hal_ctx = NULL;
 
+    hotplug_hook = NULL;
+
     if (!config_dbus_core_add_hook(&hook)) {
         LogMessage(X_ERROR, "config/hal: failed to add D-Bus hook\n");
         return 0;
diff --git a/hw/xfree86/common/xf86Config.c b/hw/xfree86/common/xf86Config.c
index 28707d8..5833687 100644
--- a/hw/xfree86/common/xf86Config.c
+++ b/hw/xfree86/common/xf86Config.c
@@ -51,6 +51,10 @@
 #include <grp.h>
 #endif
 
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <unistd.h>
+
 #include "xf86.h"
 #include "xf86Parser.h"
 #include "xf86tokens.h"
@@ -2631,3 +2635,220 @@ xf86PathIsSafe(const char *path)
 {
     return (xf86pathIsSafe(path) != 0);
 }
+
+static Bool
+device_is_duplicate(const char *config_info)
+{
+    DeviceIntPtr dev;
+
+    for (dev = inputInfo.devices; dev; dev = dev->next) {
+        if (dev->config_info && (strcmp(dev->config_info, config_info) == 0))
+            return True;
+    }
+
+    for (dev = inputInfo.off_devices; dev; dev = dev->next) {
+        if (dev->config_info && (strcmp(dev->config_info, config_info) == 0))
+            return True;
+    }
+
+    return False;
+}
+
+/* Figure out whether an InputDevice in xorg.conf will become a core device.
+ * This can happen in 3 ways:
+ * 1. The InputDevice section has Core(Pointer|Keyboard).
+ * 2. The device was given on the command line from -pointer or -keyboard.
+ * 3. The active ServerLayout section lists the InputDevice as core.
+ */
+static Bool
+device_is_core(XF86ConfInputPtr conf, IDevPtr idev)
+{
+    IDevPtr *devs;
+
+    /* Is this a core device in the configuration? */
+    if (xf86CheckBoolOption(conf->inp_option_lst, "CorePointer", FALSE))
+        return True;
+    if (xf86CheckBoolOption(conf->inp_option_lst, "CoreKeyboard", FALSE))
+        return True;
+
+    /* Was this the device given on the command line? */
+    if (xf86PointerName && strcmp(idev->identifier, xf86PointerName) == 0)
+        return True;
+    if (xf86KeyboardName && strcmp(idev->identifier, xf86KeyboardName) == 0)
+        return True;
+
+    /* Is the InputDevice specified as a core device in the active layout? */
+    for (devs = xf86ConfigLayout.inputs; devs && *devs; devs++) {
+        IDevPtr idp = *devs;
+        if (strcmp(idev->identifier, idp->identifier) != 0)
+            continue;
+        if (idp->commonOptions &&
+            (xf86CheckBoolOption(idp->commonOptions, "CorePointer", FALSE) ||
+             xf86CheckBoolOption(idp->commonOptions, "CoreKeyboard", FALSE)))
+            return True;
+        if (idp->extraOptions &&
+            (xf86CheckBoolOption(idp->extraOptions, "CorePointer", FALSE) ||
+             xf86CheckBoolOption(idp->extraOptions, "CoreKeyboard", FALSE)))
+            return True;
+    }
+
+    return False;
+}
+
+/* Add a hotplugged device from the configuration. It it's already been
+ * added or is a core device, skip it. */
+static Bool
+add_hotplug_device(XF86ConfInputPtr conf, const char *udi)
+{
+    IDevPtr idev = NULL;
+    DeviceIntPtr pdev;
+    char *config_info = NULL;
+    Bool ret = False;
+    int rc;
+
+    idev = xcalloc(1, sizeof(*idev));
+    if (!idev)
+        goto unwind;
+
+    if (!configInput(idev, conf, X_CONFIG))
+        goto unwind;
+
+    config_info = xalloc(strlen(udi) + 6); /* "xf86:" and NULL */
+    if (!config_info) {
+        xf86Msg(X_ERROR, "xf86: Could not allocate name\n");
+        goto unwind;
+    }
+    sprintf(config_info, "xf86:%s", udi);
+
+    /* Skip duplicate devices. */
+    if (device_is_duplicate(config_info)) {
+        xf86Msg(X_WARNING, "xf86: Device %s already added. Ignoring.\n",
+                idev->identifier);
+        ret = True;
+        goto unwind;
+    }
+
+    /* Skip core devices when AllowEmptyInput is false. */
+    if (!xf86Info.allowEmptyInput && device_is_core(conf, idev)) {
+        xf86MsgVerb(X_INFO, 4, "xf86: Device %s listed as core. Ignoring.\n",
+                    idev->identifier);
+        ret = True;
+        goto unwind;
+    }
+
+    /* Add the device and enable it if AutoEnableDevices is true. */
+    xf86Msg(X_INFO, "xf86: Adding input device %s\n", idev->identifier);
+    rc = xf86NewInputDevice(idev, &pdev, xf86Info.autoEnableDevices);
+    if (rc != Success) {
+        xf86Msg(X_ERROR, "xf86: xf86NewInputDevice failed (%d)\n", rc);
+        goto unwind;
+    }
+
+    /* Store the config_info so that we can skip duplicates later. */
+    for(; pdev; pdev = pdev->next) {
+        if (pdev->config_info)
+            xfree(pdev->config_info);
+        pdev->config_info = xstrdup(config_info);
+    }
+    xfree(config_info);
+
+    return True;
+
+unwind:
+    if (idev)
+        xfree(idev);
+    if (config_info)
+        xfree(config_info);
+    return ret;
+}
+
+/* See if we can find an InputDevice in the configuration whose Device
+ * matches the one being hotplugged. If so, add it. */
+Bool
+xf86HotplugAddDevice(const char *path, const char *udi)
+{
+    struct stat device_stat;
+    XF86ConfInputPtr conf;
+
+    if (!xf86configptr) {
+        xf86Msg(X_ERROR,
+                "xf86: Cannot access global config data structure\n");
+        return False;
+    }
+
+    if ((stat(path, &device_stat) < 0) || !S_ISCHR(device_stat.st_mode))
+        return False;
+
+    for (conf = xf86configptr->conf_input_lst; conf; conf = conf->list.next)
+    {
+        char *conf_path;
+        struct stat conf_stat;
+
+        if (!conf->inp_option_lst)
+            continue;
+
+        conf_path = xf86FindOptionValue(conf->inp_option_lst, "Device");
+        if (!conf_path)
+            conf_path = xf86FindOptionValue(conf->inp_option_lst, "Path");
+        if (!conf_path)
+            continue;
+
+        if ((stat(conf_path, &conf_stat) < 0) || !S_ISCHR(conf_stat.st_mode))
+            continue;
+
+        /* If both paths are character devices with equal device numbers,
+         * then we have a match. */
+        if (device_stat.st_rdev == conf_stat.st_rdev)
+            break;
+    }
+
+    if (!conf)
+        return False;
+
+    return add_hotplug_device(conf, udi);
+}
+
+static void
+remove_hotplug_device(DeviceIntPtr dev)
+{
+    xf86Msg(X_INFO, "xf86: removing device %s\n", dev->name);
+
+    /* Call PIE here so we don't try to dereference a device that's
+     * already been removed. */
+    OsBlockSignals();
+    ProcessInputEvents();
+    DeleteInputDeviceRequest(dev);
+    OsReleaseSignals();
+}
+
+Bool
+xf86HotplugRemoveDevice(const char *udi)
+{
+    Bool ret = False;
+    char *config_info;
+    DeviceIntPtr dev, next;
+
+    config_info = xalloc(strlen(udi) + 6); /* "xf86:" and NULL */
+    if (!config_info) {
+        xf86Msg(X_ERROR, "xf86: Could not allocate name\n");
+        goto unwind;
+    }
+    sprintf(config_info, "xf86:%s", udi);
+
+    for (dev = inputInfo.devices; dev; dev = next) {
+        next = dev->next;
+        if (dev->config_info && strcmp(config_info, dev->config_info) == 0)
+            remove_hotplug_device(dev);
+    }
+
+    for (dev = inputInfo.off_devices; dev; dev = next) {
+        next = dev->next;
+        if (dev->config_info && strcmp(config_info, dev->config_info) == 0)
+            remove_hotplug_device(dev);
+    }
+
+unwind:
+    if (config_info)
+        xfree(config_info);
+    return ret;
+}
diff --git a/hw/xfree86/common/xf86Config.h b/hw/xfree86/common/xf86Config.h
index a174e46..3d10825 100644
--- a/hw/xfree86/common/xf86Config.h
+++ b/hw/xfree86/common/xf86Config.h
@@ -71,4 +71,7 @@ GDevPtr autoConfigDevice(GDevPtr preconf_device);
 char* chooseVideoDriver(void);
 int xchomp(char *line);
 
+Bool xf86HotplugAddDevice(const char *path, const char *udi);
+Bool xf86HotplugRemoveDevice(const char *udi);
+
 #endif /* _xf86_config_h */
diff --git a/hw/xfree86/common/xf86Init.c b/hw/xfree86/common/xf86Init.c
index 01acb8e..d209823 100644
--- a/hw/xfree86/common/xf86Init.c
+++ b/hw/xfree86/common/xf86Init.c
@@ -88,6 +88,10 @@
 #include "dpmsproc.h"
 #endif
 
+#ifdef CONFIG_HAL
+#include <hotplug.h>
+#endif
+
 #include <pciaccess.h>
 #include "Pci.h"
 #include "xf86Bus.h"
@@ -121,6 +125,13 @@ static int numFormats = 6;
 #endif
 static Bool formatsDone = FALSE;
 
+#ifdef CONFIG_HAL
+static struct config_hal_hotplug_hook hotplug_hook = {
+    .add    = xf86HotplugAddDevice,
+    .remove = xf86HotplugRemoveDevice,
+};
+#endif
+
 #ifndef OSNAME
 #define OSNAME " unknown"
 #endif
@@ -837,6 +848,11 @@ InitOutput(ScreenInfo *pScreenInfo, int argc, char **argv)
       xfree(modulelist);
     }
 
+#ifdef CONFIG_HAL
+    /* Register the hal hotplug handler. */
+    config_hal_hotplug_set_hook(&hotplug_hook);
+#endif
+
     /* Load all input driver modules specified in the config file. */
     if ((modulelist = xf86InputDriverlistFromConfig())) {
       xf86LoadModules(modulelist, NULL);
@@ -1392,6 +1408,10 @@ ddxGiveUp()
     xf86OSPMClose = NULL;
 #endif
 
+#ifdef CONFIG_HAL
+    config_hal_hotplug_clear_hook();
+#endif
+
     xf86AccessLeaveState();
 
     for (i = 0; i < xf86NumScreens; i++) {
diff --git a/hw/xfree86/common/xf86Xinput.c b/hw/xfree86/common/xf86Xinput.c
index 89a27c7..b87eeaa 100644
--- a/hw/xfree86/common/xf86Xinput.c
+++ b/hw/xfree86/common/xf86Xinput.c
@@ -654,6 +654,7 @@ DeleteInputDeviceRequest(DeviceIntPtr pDev)
     IDevRec *idev;
     IDevPtr *it;
     Bool isMaster = pDev->isMaster;
+    Bool isHalDev;
 
     if (pInfo) /* need to get these before RemoveDevice */
     {
@@ -661,6 +662,9 @@ DeleteInputDeviceRequest(DeviceIntPtr pDev)
         idev = pInfo->conf_idev;
     }
 
+    isHalDev = (pDev->config_info &&
+                strncmp(pDev->config_info, "hal:", 4) == 0);
+
     OsBlockSignals();
     RemoveDevice(pDev);
 
@@ -671,12 +675,8 @@ DeleteInputDeviceRequest(DeviceIntPtr pDev)
         else
             xf86DeleteInput(pInfo, 0);
 
-        /* devices added through HAL aren't in the config layout */
-        it = xf86ConfigLayout.inputs;
-        while(*it && *it != idev)
-            it++;
-
-        if (!(*it)) /* end of list, not in the layout */
+        /* devices added through HAL aren't in the configuration */
+        if (isHalDev)
         {
             xfree(idev->driver);
             xfree(idev->identifier);
diff --git a/include/hotplug.h b/include/hotplug.h
index b4f1bb6..ce53113 100644
--- a/include/hotplug.h
+++ b/include/hotplug.h
@@ -29,4 +29,18 @@
 void config_init(void);
 void config_fini(void);
 
+#ifdef CONFIG_HAL
+typedef Bool (*config_hal_hotplug_add_hook)(const char *path,
+                                            const char *udi);
+typedef Bool (*config_hal_hotplug_remove_hook)(const char *udi);
+
+struct config_hal_hotplug_hook {
+    config_hal_hotplug_add_hook add;
+    config_hal_hotplug_remove_hook remove;
+};
+
+void config_hal_hotplug_set_hook(struct config_hal_hotplug_hook *hook);
+void config_hal_hotplug_clear_hook(void);
+#endif
+
 #endif /* HOTPLUG_H */
-- 
1.5.6.5




More information about the xorg mailing list