[PATCH 1/2] config: Replace OdevAttributes linked list with struct
Keith Packard
keithp at keithp.com
Tue Jul 15 17:31:58 PDT 2014
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 at keithp.com>
---
config/config.c | 161 ++---------------------------
config/udev.c | 10 +-
hw/xfree86/common/xf86platformBus.c | 58 ++---------
hw/xfree86/common/xf86platformBus.h | 27 ++---
hw/xfree86/os-support/linux/lnx_platform.c | 52 +++-------
include/hotplug.h | 69 +++++--------
6 files changed, 80 insertions(+), 297 deletions(-)
diff --git a/config/config.c b/config/config.c
index a26d835..b5d634b 100644
--- a/config/config.c
+++ b/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;
-
- attriblist = XNFalloc(sizeof(struct OdevAttributes));
- 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;
+ struct OdevAttributes *attribs = XNFcalloc(sizeof (struct OdevAttributes));
+ attribs->fd = -1;
+ return attribs;
}
void
config_odev_free_attributes(struct OdevAttributes *attribs)
{
- struct OdevAttribute *iter, *safe;
- int major = 0, minor = 0, fd = -1;
-
- xorg_list_for_each_entry_safe(iter, safe, &attribs->list, member) {
- switch (iter->attrib_id) {
- case ODEV_ATTRIB_MAJOR: major = iter->attrib_value; break;
- case ODEV_ATTRIB_MINOR: minor = iter->attrib_value; break;
- 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);
+ if (attribs->fd != -1)
+ systemd_logind_release_fd(attribs->major, attribs->minor, attribs->fd);
+ free(attribs->path);
+ free(attribs->syspath);
+ free(attribs->busid);
+ free(attribs->driver);
+ free(attribs);
}
diff --git a/config/udev.c b/config/udev.c
index a1b72c1..1e4a9d7 100644
--- a/config/udev.c
+++ b/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);
- config_odev_add_attribute(attribs, ODEV_ATTRIB_SYSPATH, syspath);
- config_odev_add_int_attribute(attribs, ODEV_ATTRIB_MAJOR, major);
- config_odev_add_int_attribute(attribs, ODEV_ATTRIB_MINOR, minor);
+ attribs->path = XNFstrdup(path);
+ attribs->syspath = XNFstrdup(syspath);
+ attribs->major = major;
+ attribs->minor = minor;
/* ownership of attribs is passed to probe layer */
probe_callback(attribs);
diff --git a/hw/xfree86/common/xf86platformBus.c b/hw/xfree86/common/xf86platformBus.c
index eb1a3fb..22e4603 100644
--- a/hw/xfree86/common/xf86platformBus.c
+++ b/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));
@@ -86,44 +86,6 @@ xf86_remove_platform_device(int dev_index)
}
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)
{
return (xf86_platform_devices[index].flags & XF86_PDEV_UNOWNED) ?
@@ -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_minor = xf86_get_platform_int_attrib(i, ODEV_ATTRIB_MINOR, 0);
+ attr_major = xf86_platform_odev_attributes(i)->major;
+ 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);
- major = xf86_get_platform_device_int_attrib(dev, ODEV_ATTRIB_MAJOR, 0);
- minor = xf86_get_platform_device_int_attrib(dev, ODEV_ATTRIB_MINOR, 0);
+ fd = dev->attribs->fd;
+ major = dev->attribs->major;
+ 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;
}
diff --git a/hw/xfree86/common/xf86platformBus.h b/hw/xfree86/common/xf86platformBus.h
index 5dee4e0..823a10c 100644
--- a/hw/xfree86/common/xf86platformBus.h
+++ b/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 *
-xf86_get_platform_device_attrib(struct xf86_platform_device *device, int attrib_id);
-extern _X_EXPORT int
-xf86_get_platform_device_int_attrib(struct xf86_platform_device *device, int attrib_id, int def);
+static inline struct OdevAttributes *
+xf86_platform_device_odev_attributes(struct xf86_platform_device *device)
+{
+ 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);
diff --git a/hw/xfree86/os-support/linux/lnx_platform.c b/hw/xfree86/os-support/linux/lnx_platform.c
index d660761..1d145b3 100644
--- a/hw/xfree86/os-support/linux/lnx_platform.c
+++ b/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);
- minor = config_odev_get_int_attribute(attribs, ODEV_ATTRIB_MINOR, 0);
+ major = attribs->major;
+ 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,
- ODEV_ATTRIB_BUSID, buf);
+ xf86_platform_odev_attributes(delayed_index)->busid = XNFstrdup(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,
- v->name);
+ xf86_platform_odev_attributes(delayed_index)->driver = XNFstrdup(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 = NULL;
+ const char *syspath = device->attribs->syspath;
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;
- dpath = xf86_get_platform_attrib(index, ODEV_ATTRIB_PATH);
+ char *dpath = attribs->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;
- dpath = xf86_get_platform_attrib(i, ODEV_ATTRIB_PATH);
+ char *dpath = xf86_platform_odev_attributes(i)->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 (attrib->attrib_id == ODEV_ATTRIB_SYSPATH) {
- syspath = attrib->attrib_name;
- break;
- }
- }
+ if (!syspath)
+ goto out;
for (index = 0; index < xf86_num_platform_devices; index++) {
- char *dspath;
- dspath = xf86_get_platform_attrib(index, ODEV_ATTRIB_SYSPATH);
- if (!strcmp(syspath, dspath))
+ char *dspath = xf86_platform_odev_attributes(index)->syspath;
+ if (dspath && !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
diff --git a/include/hotplug.h b/include/hotplug.h
index 4c2fa97..6fe76c8 100644
--- a/include/hotplug.h
+++ b/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 };
-
-struct OdevAttribute {
- struct xorg_list member;
- int attrib_id;
- union {
- char *attrib_name;
- int attrib_value;
- };
- int attrib_type;
-};
+/* Bump this each time you add something to the struct
+ * so that drivers can easily tell what is available
+ */
+#define ODEV_ATTRIBUTES_VERSION 1
struct OdevAttributes {
- struct xorg_list list;
-};
+ /* path to kernel device node - Linux e.g. /dev/dri/card0 */
+ char *path;
-/* Note starting with xserver 1.16 this function never fails */
-struct OdevAttributes *
-config_odev_allocate_attribute_list(void);
+ /* system device path - Linux e.g. /sys/devices/pci0000:00/0000:00:01.0/0000:01:00.0/drm/card1 */
+ char *syspath;
-void
-config_odev_free_attribute_list(struct OdevAttributes *attribs);
+ /* DRI-style bus id */
+ char *busid;
-/* Note starting with xserver 1.16 this function never fails */
-Bool
-config_odev_add_attribute(struct OdevAttributes *attribs, int attrib,
- const char *attrib_name);
+ /* Server managed FD */
+ int fd;
-char *
-config_odev_get_attribute(struct OdevAttributes *attribs, int attrib_id);
+ /* Major number of the device node pointed to by ODEV_ATTRIB_PATH */
+ int major;
-/* Note starting with xserver 1.16 this function never fails */
-Bool
-config_odev_add_int_attribute(struct OdevAttributes *attribs, int attrib,
- int attrib_value);
+ /* Minor number of the device node pointed to by ODEV_ATTRIB_PATH */
+ int minor;
+
+ /* kernel driver name */
+ char *driver;
+};
-int
-config_odev_get_int_attribute(struct OdevAttributes *attribs, int attrib,
- int def);
+/* Note starting with xserver 1.16 this function never fails */
+struct OdevAttributes *
+config_odev_allocate_attributes(void);
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);
--
2.0.1
More information about the xorg-devel
mailing list