config: Replace OdevAttributes linked list with struct

OdevAttributes are a fixed set of values with known types; instead of
storing them in a linked list and requiring accessor/settor functions,
replace the list header, struct OdevAttributes, with a struct that
directly contains the values. This provides for compile-time
typechecking of the values, eliminates a significant amount of code
and generally simplifies using this datatype.

Signed-off-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>

config/config.c

@@ -128,160 +128,21 @@ device_is_duplicate(const char *config_info)
 }
 
 struct OdevAttributes *
-config_odev_allocate_attribute_list(void)
+config_odev_allocate_attributes(void)
 {
-    struct OdevAttributes *attriblist;
+    struct OdevAttributes *attribs = XNFcalloc(sizeof (struct OdevAttributes));
-
+    attribs->fd = -1;
-    attriblist = XNFalloc(sizeof(struct OdevAttributes));
+    return attribs;
-    xorg_list_init(&attriblist->list);
-    return attriblist;
-}
-
-void
-config_odev_free_attribute_list(struct OdevAttributes *attribs)
-{
-    config_odev_free_attributes(attribs);
-    free(attribs);
-}
-
-static struct OdevAttribute *
-config_odev_find_attribute(struct OdevAttributes *attribs, int attrib_id)
-{
-    struct OdevAttribute *oa;
-
-    xorg_list_for_each_entry(oa, &attribs->list, member) {
-        if (oa->attrib_id == attrib_id)
-          return oa;
-    }
-    return NULL;
-}
-
-static struct OdevAttribute *
-config_odev_find_or_add_attribute(struct OdevAttributes *attribs, int attrib)
-{
-    struct OdevAttribute *oa;
-
-    oa = config_odev_find_attribute(attribs, attrib);
-    if (oa)
-        return oa;
-
-    oa = XNFcalloc(sizeof(struct OdevAttribute));
-    oa->attrib_id = attrib;
-    xorg_list_append(&oa->member, &attribs->list);
-
-    return oa;
-}
-
-static int config_odev_get_attribute_type(int attrib)
-{
-    switch (attrib) {
-    case ODEV_ATTRIB_PATH:
-    case ODEV_ATTRIB_SYSPATH:
-    case ODEV_ATTRIB_BUSID:
-        return ODEV_ATTRIB_STRING;
-    case ODEV_ATTRIB_FD:
-    case ODEV_ATTRIB_MAJOR:
-    case ODEV_ATTRIB_MINOR:
-        return ODEV_ATTRIB_INT;
-    case ODEV_ATTRIB_DRIVER:
-        return ODEV_ATTRIB_STRING;
-    default:
-        LogMessage(X_ERROR, "Error %s called for unknown attribute %d\n",
-                   __func__, attrib);
-        return ODEV_ATTRIB_UNKNOWN;
-    }
-}
-
-Bool
-config_odev_add_attribute(struct OdevAttributes *attribs, int attrib,
-                          const char *attrib_name)
-{
-    struct OdevAttribute *oa;
-
-    if (config_odev_get_attribute_type(attrib) != ODEV_ATTRIB_STRING) {
-        LogMessage(X_ERROR, "Error %s called for non string attrib %d\n",
-                   __func__, attrib);
-        return FALSE;
-    }
-
-    oa = config_odev_find_or_add_attribute(attribs, attrib);
-    free(oa->attrib_name);
-    oa->attrib_name = XNFstrdup(attrib_name);
-    oa->attrib_type = ODEV_ATTRIB_STRING;
-    return TRUE;
-}
-
-Bool
-config_odev_add_int_attribute(struct OdevAttributes *attribs, int attrib,
-                              int attrib_value)
-{
-    struct OdevAttribute *oa;
-
-    if (config_odev_get_attribute_type(attrib) != ODEV_ATTRIB_INT) {
-        LogMessage(X_ERROR, "Error %s called for non integer attrib %d\n",
-                   __func__, attrib);
-        return FALSE;
-    }
-
-    oa = config_odev_find_or_add_attribute(attribs, attrib);
-    oa->attrib_value = attrib_value;
-    oa->attrib_type = ODEV_ATTRIB_INT;
-    return TRUE;
-}
-
-char *
-config_odev_get_attribute(struct OdevAttributes *attribs, int attrib_id)
-{
-    struct OdevAttribute *oa;
-
-    oa = config_odev_find_attribute(attribs, attrib_id);
-    if (!oa)
-        return NULL;
-
-    if (oa->attrib_type != ODEV_ATTRIB_STRING) {
-        LogMessage(X_ERROR, "Error %s called for non string attrib %d\n",
-                   __func__, attrib_id);
-        return NULL;
-    }
-    return oa->attrib_name;
-}
-
-int
-config_odev_get_int_attribute(struct OdevAttributes *attribs, int attrib_id, int def)
-{
-    struct OdevAttribute *oa;
-
-    oa = config_odev_find_attribute(attribs, attrib_id);
-    if (!oa)
-        return def;
-
-    if (oa->attrib_type != ODEV_ATTRIB_INT) {
-        LogMessage(X_ERROR, "Error %s called for non integer attrib %d\n",
-                   __func__, attrib_id);
-        return def;
-    }
-
-    return oa->attrib_value;
 }
 
 void
 config_odev_free_attributes(struct OdevAttributes *attribs)
 {
-    struct OdevAttribute *iter, *safe;
+    if (attribs->fd != -1)
-    int major = 0, minor = 0, fd = -1;
+        systemd_logind_release_fd(attribs->major, attribs->minor, attribs->fd);
-
+    free(attribs->path);
-    xorg_list_for_each_entry_safe(iter, safe, &attribs->list, member) {
+    free(attribs->syspath);
-        switch (iter->attrib_id) {
+    free(attribs->busid);
-        case ODEV_ATTRIB_MAJOR: major = iter->attrib_value; break;
+    free(attribs->driver);
-        case ODEV_ATTRIB_MINOR: minor = iter->attrib_value; break;
+    free(attribs);
-        case ODEV_ATTRIB_FD: fd = iter->attrib_value; break;
-        }
-        xorg_list_del(&iter->member);
-        if (iter->attrib_type == ODEV_ATTRIB_STRING)
-            free(iter->attrib_name);
-        free(iter);
-    }
-
-    if (fd != -1)
-        systemd_logind_release_fd(major, minor, fd);
 }

config/udev.c

@@ -462,12 +462,12 @@ config_udev_odev_setup_attribs(const char *path, const char *syspath,
                                int major, int minor,
                                config_odev_probe_proc_ptr probe_callback)
 {
-    struct OdevAttributes *attribs = config_odev_allocate_attribute_list();
+    struct OdevAttributes *attribs = config_odev_allocate_attributes();
 
-    config_odev_add_attribute(attribs, ODEV_ATTRIB_PATH, path);
+    attribs->path = XNFstrdup(path);
-    config_odev_add_attribute(attribs, ODEV_ATTRIB_SYSPATH, syspath);
+    attribs->syspath = XNFstrdup(syspath);
-    config_odev_add_int_attribute(attribs, ODEV_ATTRIB_MAJOR, major);
+    attribs->major = major;
-    config_odev_add_int_attribute(attribs, ODEV_ATTRIB_MINOR, minor);
+    attribs->minor = minor;
 
     /* ownership of attribs is passed to probe layer */
     probe_callback(attribs);

hw/xfree86/common/xf86platformBus.c

@@ -77,7 +77,7 @@ xf86_remove_platform_device(int dev_index)
 {
     int j;
 
-    config_odev_free_attribute_list(xf86_platform_devices[dev_index].attribs);
+    config_odev_free_attributes(xf86_platform_devices[dev_index].attribs);
 
     for (j = dev_index; j < xf86_num_platform_devices - 1; j++)
         memcpy(&xf86_platform_devices[j], &xf86_platform_devices[j + 1], sizeof(struct xf86_platform_device));
@@ -85,44 +85,6 @@ xf86_remove_platform_device(int dev_index)
     return 0;
 }
 
-Bool
-xf86_add_platform_device_attrib(int index, int attrib_id, char *attrib_name)
-{
-    struct xf86_platform_device *device = &xf86_platform_devices[index];
-
-    return config_odev_add_attribute(device->attribs, attrib_id, attrib_name);
-}
-
-Bool
-xf86_add_platform_device_int_attrib(int index, int attrib_id, int attrib_value)
-{
-    return config_odev_add_int_attribute(xf86_platform_devices[index].attribs, attrib_id, attrib_value);
-}
-
-char *
-xf86_get_platform_attrib(int index, int attrib_id)
-{
-    return config_odev_get_attribute(xf86_platform_devices[index].attribs, attrib_id);
-}
-
-char *
-xf86_get_platform_device_attrib(struct xf86_platform_device *device, int attrib_id)
-{
-    return config_odev_get_attribute(device->attribs, attrib_id);
-}
-
-int
-xf86_get_platform_int_attrib(int index, int attrib_id, int def)
-{
-    return config_odev_get_int_attribute(xf86_platform_devices[index].attribs, attrib_id, def);
-}
-
-int
-xf86_get_platform_device_int_attrib(struct xf86_platform_device *device, int attrib_id, int def)
-{
-    return config_odev_get_int_attribute(device->attribs, attrib_id, def);
-}
-
 Bool
 xf86_get_platform_device_unowned(int index)
 {
@@ -136,8 +98,8 @@ xf86_find_platform_device_by_devnum(int major, int minor)
     int i, attr_major, attr_minor;
 
     for (i = 0; i < xf86_num_platform_devices; i++) {
-        attr_major = xf86_get_platform_int_attrib(i, ODEV_ATTRIB_MAJOR, 0);
+        attr_major = xf86_platform_odev_attributes(i)->major;
-        attr_minor = xf86_get_platform_int_attrib(i, ODEV_ATTRIB_MINOR, 0);
+        attr_minor = xf86_platform_odev_attributes(i)->minor;
         if (attr_major == major && attr_minor == minor)
             return &xf86_platform_devices[i];
     }
@@ -240,7 +202,7 @@ MatchToken(const char *value, struct xorg_list *patterns,
 static Bool
 OutputClassMatches(const XF86ConfOutputClassPtr oclass, int index)
 {
-    char *driver = xf86_get_platform_attrib(index, ODEV_ATTRIB_DRIVER);
+    char *driver = xf86_platform_odev_attributes(index)->driver;
 
     if (!MatchToken(driver, &oclass->match_driver, strcmp))
         return FALSE;
@@ -259,7 +221,7 @@ xf86OutputClassDriverList(int index, char *matches[], int nmatches)
 
     for (cl = xf86configptr->conf_outputclass_lst; cl; cl = cl->list.next) {
         if (OutputClassMatches(cl, index)) {
-            char *path = xf86_get_platform_attrib(index, ODEV_ATTRIB_PATH);
+            char *path = xf86_platform_odev_attributes(index)->path;
 
             xf86Msg(X_INFO, "Applying OutputClass \"%s\" to %s\n",
                     cl->identifier, path);
@@ -324,7 +286,7 @@ xf86platformProbe(void)
     }
 
     for (i = 0; i < xf86_num_platform_devices; i++) {
-        char *busid = xf86_get_platform_attrib(i, ODEV_ATTRIB_BUSID);
+        char *busid = xf86_platform_odev_attributes(i)->busid;
 
         if (pci && (strncmp(busid, "pci:", 4) == 0)) {
             platform_find_pci_info(&xf86_platform_devices[i], busid);
@@ -412,11 +374,11 @@ static Bool doPlatformProbe(struct xf86_platform_device *dev, DriverPtr drvp,
     if (entity != -1) {
         if ((dev->flags & XF86_PDEV_SERVER_FD) && (!drvp->driverFunc ||
                 !drvp->driverFunc(NULL, SUPPORTS_SERVER_FDS, NULL))) {
-            fd = xf86_get_platform_device_int_attrib(dev, ODEV_ATTRIB_FD, -1);
+            fd = dev->attribs->fd;
-            major = xf86_get_platform_device_int_attrib(dev, ODEV_ATTRIB_MAJOR, 0);
+            major = dev->attribs->major;
-            minor = xf86_get_platform_device_int_attrib(dev, ODEV_ATTRIB_MINOR, 0);
+            minor = dev->attribs->minor;
             systemd_logind_release_fd(major, minor, fd);
-            config_odev_add_int_attribute(dev->attribs, ODEV_ATTRIB_FD, -1);
+            dev->attribs->fd = -1;
             dev->flags &= ~XF86_PDEV_SERVER_FD;
         }
 

hw/xfree86/common/xf86platformBus.h

@@ -45,31 +45,32 @@ int xf86platformProbeDev(DriverPtr drvp);
 extern int xf86_num_platform_devices;
 extern struct xf86_platform_device *xf86_platform_devices;
 
-extern char *
-xf86_get_platform_attrib(int index, int attrib_id);
-extern int
-xf86_get_platform_int_attrib(int index, int attrib_id, int def);
 extern int
 xf86_add_platform_device(struct OdevAttributes *attribs, Bool unowned);
 extern int
 xf86_remove_platform_device(int dev_index);
 extern Bool
 xf86_get_platform_device_unowned(int index);
-/* Note starting with xserver 1.16 these 2 functions never fail */
-extern Bool
-xf86_add_platform_device_attrib(int index, int attrib_id, char *attrib_str);
-extern Bool
-xf86_add_platform_device_int_attrib(int index, int attrib_id, int attrib_value);
 
 extern int
 xf86platformAddDevice(int index);
 extern void
 xf86platformRemoveDevice(int index);
 
-extern _X_EXPORT char *
+static inline struct OdevAttributes *
-xf86_get_platform_device_attrib(struct xf86_platform_device *device, int attrib_id);
+xf86_platform_device_odev_attributes(struct xf86_platform_device *device)
-extern _X_EXPORT int
+{
-xf86_get_platform_device_int_attrib(struct xf86_platform_device *device, int attrib_id, int def);
+    return device->attribs;
+}
+
+static inline struct OdevAttributes *
+xf86_platform_odev_attributes(int index)
+{
+    struct xf86_platform_device *device = &xf86_platform_devices[index];
+
+    return device->attribs;
+}
+
 extern _X_EXPORT Bool
 xf86PlatformDeviceCheckBusID(struct xf86_platform_device *device, const char *busid);
 

hw/xfree86/os-support/linux/lnx_platform.c

@@ -30,8 +30,8 @@ get_drm_info(struct OdevAttributes *attribs, char *path, int delayed_index)
     int err = 0;
     Bool paused, server_fd = FALSE;
 
-    major = config_odev_get_int_attribute(attribs, ODEV_ATTRIB_MAJOR, 0);
+    major = attribs->major;
-    minor = config_odev_get_int_attribute(attribs, ODEV_ATTRIB_MINOR, 0);
+    minor = attribs->minor;
 
     fd = systemd_logind_take_fd(major, minor, path, &paused);
     if (fd != -1) {
@@ -41,7 +41,7 @@ get_drm_info(struct OdevAttributes *attribs, char *path, int delayed_index)
             systemd_logind_release_fd(major, minor, -1);
             return FALSE;
         }
-        config_odev_add_int_attribute(attribs, ODEV_ATTRIB_FD, fd);
+        attribs->fd = fd;
         server_fd = TRUE;
     }
 
@@ -73,8 +73,7 @@ get_drm_info(struct OdevAttributes *attribs, char *path, int delayed_index)
         xf86_platform_devices[delayed_index].flags |= XF86_PDEV_SERVER_FD;
 
     buf = drmGetBusid(fd);
-    xf86_add_platform_device_attrib(delayed_index,
+    xf86_platform_odev_attributes(delayed_index)->busid = XNFstrdup(buf);
-                                    ODEV_ATTRIB_BUSID, buf);
     drmFreeBusid(buf);
 
     v = drmGetVersion(fd);
@@ -83,8 +82,7 @@ get_drm_info(struct OdevAttributes *attribs, char *path, int delayed_index)
         goto out;
     }
 
-    xf86_add_platform_device_attrib(delayed_index, ODEV_ATTRIB_DRIVER,
+    xf86_platform_odev_attributes(delayed_index)->driver = XNFstrdup(v->name);
-                                    v->name);
     drmFreeVersion(v);
 
 out:
@@ -96,16 +94,9 @@ out:
 Bool
 xf86PlatformDeviceCheckBusID(struct xf86_platform_device *device, const char *busid)
 {
-    struct OdevAttribute *attrib;
+    const char *syspath = device->attribs->syspath;
-    const char *syspath = NULL;
     BusType bustype;
     const char *id;
-    xorg_list_for_each_entry(attrib, &device->attribs->list, member) {
-        if (attrib->attrib_id == ODEV_ATTRIB_SYSPATH) {
-            syspath = attrib->attrib_name;
-            break;
-        }
-    }
 
     if (!syspath)
         return FALSE;
@@ -138,8 +129,7 @@ void
 xf86PlatformReprobeDevice(int index, struct OdevAttributes *attribs)
 {
     Bool ret;
-    char *dpath;
+    char *dpath = attribs->path;
-    dpath = xf86_get_platform_attrib(index, ODEV_ATTRIB_PATH);
 
     ret = get_drm_info(attribs, dpath, index);
     if (ret == FALSE) {
@@ -155,18 +145,16 @@ void
 xf86PlatformDeviceProbe(struct OdevAttributes *attribs)
 {
     int i;
-    char *path = NULL;
+    char *path = attribs->path;
     Bool ret;
 
-    path = config_odev_get_attribute(attribs, ODEV_ATTRIB_PATH);
     if (!path)
         goto out_free;
 
     for (i = 0; i < xf86_num_platform_devices; i++) {
-        char *dpath;
+        char *dpath = xf86_platform_odev_attributes(i)->path;
-        dpath = xf86_get_platform_attrib(i, ODEV_ATTRIB_PATH);
 
-        if (!strcmp(path, dpath))
+        if (dpath && !strcmp(path, dpath))
             break;
     }
 
@@ -189,7 +177,7 @@ xf86PlatformDeviceProbe(struct OdevAttributes *attribs)
     return;
 
 out_free:
-    config_odev_free_attribute_list(attribs);
+    config_odev_free_attributes(attribs);
 }
 
 void NewGPUDeviceRequest(struct OdevAttributes *attribs)
@@ -214,21 +202,15 @@ void NewGPUDeviceRequest(struct OdevAttributes *attribs)
 
 void DeleteGPUDeviceRequest(struct OdevAttributes *attribs)
 {
-    struct OdevAttribute *attrib;
     int index;
-    char *syspath = NULL;
+    char *syspath = attribs->syspath;
 
-    xorg_list_for_each_entry(attrib, &attribs->list, member) {
+    if (!syspath)
-        if (attrib->attrib_id == ODEV_ATTRIB_SYSPATH) {
+        goto out;
-            syspath = attrib->attrib_name;
-            break;
-        }
-    }
 
     for (index = 0; index < xf86_num_platform_devices; index++) {
-        char *dspath;
+        char *dspath = xf86_platform_odev_attributes(index)->syspath;
-        dspath = xf86_get_platform_attrib(index, ODEV_ATTRIB_SYSPATH);
+        if (dspath && !strcmp(syspath, dspath))
-        if (!strcmp(syspath, dspath))
             break;
     }
 
@@ -242,7 +224,7 @@ void DeleteGPUDeviceRequest(struct OdevAttributes *attribs)
     else
             xf86platformRemoveDevice(index);
 out:
-    config_odev_free_attribute_list(attribs);
+    config_odev_free_attributes(attribs);
 }
 
 #endif

include/hotplug.h

@@ -32,64 +32,41 @@ extern _X_EXPORT void config_pre_init(void);
 extern _X_EXPORT void config_init(void);
 extern _X_EXPORT void config_fini(void);
 
-enum { ODEV_ATTRIB_UNKNOWN = -1, ODEV_ATTRIB_STRING = 0, ODEV_ATTRIB_INT };
+/* Bump this each time you add something to the struct
-
+ * so that drivers can easily tell what is available
-struct OdevAttribute {
+ */
-    struct xorg_list member;
+#define ODEV_ATTRIBUTES_VERSION         1
-    int attrib_id;
-    union {
-        char *attrib_name;
-        int attrib_value;
-    };
-    int attrib_type;
-};
 
 struct OdevAttributes {
-    struct xorg_list list;
+    /* path to kernel device node - Linux e.g. /dev/dri/card0 */
+    char        *path;
+
+    /* system device path - Linux e.g. /sys/devices/pci0000:00/0000:00:01.0/0000:01:00.0/drm/card1 */
+    char        *syspath;
+
+    /* DRI-style bus id */
+    char        *busid;
+
+    /* Server managed FD */
+    int         fd;
+
+    /* Major number of the device node pointed to by ODEV_ATTRIB_PATH */
+    int         major;
+
+    /* Minor number of the device node pointed to by ODEV_ATTRIB_PATH */
+    int         minor;
+
+    /* kernel driver name */
+    char        *driver;
 };
 
 /* Note starting with xserver 1.16 this function never fails */
 struct OdevAttributes *
-config_odev_allocate_attribute_list(void);
+config_odev_allocate_attributes(void);
-
-void
-config_odev_free_attribute_list(struct OdevAttributes *attribs);
-
-/* Note starting with xserver 1.16 this function never fails */
-Bool
-config_odev_add_attribute(struct OdevAttributes *attribs, int attrib,
-                          const char *attrib_name);
-
-char *
-config_odev_get_attribute(struct OdevAttributes *attribs, int attrib_id);
-
-/* Note starting with xserver 1.16 this function never fails */
-Bool
-config_odev_add_int_attribute(struct OdevAttributes *attribs, int attrib,
-                              int attrib_value);
-
-int
-config_odev_get_int_attribute(struct OdevAttributes *attribs, int attrib,
-                              int def);
 
 void
 config_odev_free_attributes(struct OdevAttributes *attribs);
 
-/* path to kernel device node - Linux e.g. /dev/dri/card0 */
-#define ODEV_ATTRIB_PATH 1
-/* system device path - Linux e.g. /sys/devices/pci0000:00/0000:00:01.0/0000:01:00.0/drm/card1 */
-#define ODEV_ATTRIB_SYSPATH 2
-/* DRI-style bus id */
-#define ODEV_ATTRIB_BUSID 3
-/* Server managed FD */
-#define ODEV_ATTRIB_FD 4
-/* Major number of the device node pointed to by ODEV_ATTRIB_PATH */
-#define ODEV_ATTRIB_MAJOR 5
-/* Minor number of the device node pointed to by ODEV_ATTRIB_PATH */
-#define ODEV_ATTRIB_MINOR 6
-/* kernel driver name */
-#define ODEV_ATTRIB_DRIVER 7
-
 typedef void (*config_odev_probe_proc_ptr)(struct OdevAttributes *attribs);
 void config_odev_probe(config_odev_probe_proc_ptr probe_callback);