input: make InputOption opaque, provide interface functions.

InputOptions is not switched to use struct list for a future patch to unify
it with the XF86OptionRec.

Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Dan Nicholson <dbn.lists@gmail.com>

config/config-backends.h

@@ -27,10 +27,10 @@
 #include <dix-config.h>
 #endif
 #include "input.h"
+#include "list.h"
 
 void remove_devices(const char *backend, const char *config_info);
 BOOL device_is_duplicate(const char *config_info);
-InputOption* add_option(InputOption **options, const char *key, const char *value);
 
 #ifdef CONFIG_UDEV
 int config_udev_init(void);

config/config.c

@@ -122,25 +122,3 @@ device_is_duplicate(const char *config_info)
     return FALSE;
 }
 
-/**
- * Allocate a new option and append to the list.
- *
- * @return A pointer to the newly allocated InputOption struct.
- */
-InputOption*
-add_option(InputOption **options, const char *key, const char *value)
-{
-    if (!value || *value == '\0')
-        return NULL;
-
-    for (; *options; options = &(*options)->next)
-        ;
-    *options = calloc(sizeof(**options), 1);
-    if (!*options) /* Yeesh. */
-        return NULL;
-    (*options)->key = strdup(key);
-    (*options)->value = strdup(value);
-    (*options)->next = NULL;
-
-    return *options;
-}

config/dbus.c

@@ -68,8 +68,7 @@ static int
 add_device(DBusMessage *message, DBusMessage *reply, DBusError *error)
 {
     DBusMessageIter iter, reply_iter, subiter;
-    InputOption *tmpo = NULL, *options = NULL;
+    InputOption *input_options = NULL;
-    char *tmp = NULL;
     int ret, err;
     DeviceIntPtr dev = NULL;
 
@@ -80,7 +79,8 @@ add_device(DBusMessage *message, DBusMessage *reply, DBusError *error)
         MALFORMED_MESSAGE();
     }
 
-    if (!add_option(&options, "_source", "client/dbus")) {
+    input_options = input_option_new(input_options, "_source", "client/dbus");
+    if (!input_options) {
         ErrorF("[config/dbus] couldn't allocate first key/value pair\n");
         ret = BadAlloc;
         goto unwind;
@@ -88,36 +88,22 @@ add_device(DBusMessage *message, DBusMessage *reply, DBusError *error)
 
     /* signature should be [ss][ss]... */
     while (dbus_message_iter_get_arg_type(&iter) == DBUS_TYPE_ARRAY) {
-        tmpo = calloc(sizeof(*tmpo), 1);
+        char *key, *value;
-        if (!tmpo) {
-            ErrorF("[config/dbus] couldn't allocate option\n");
-            ret = BadAlloc;
-            goto unwind;
-        }
-        tmpo->next = options;
-        options = tmpo;
-
         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);
+        dbus_message_iter_get_basic(&subiter, &key);
-        if (!tmp)
+        if (!key)
             MALFORMED_MESSAGE();
         /* The _ prefix refers to internal settings, and may not be given by
          * the client. */
-        if (tmp[0] == '_') {
+        if (key[0] == '_') {
             ErrorF("[config/dbus] attempted subterfuge: option name %s given\n",
-                   tmp);
+                   key);
             MALFORMED_MESSAGE();
         }
-        options->key = strdup(tmp);
-        if (!options->key) {
-            ErrorF("[config/dbus] couldn't duplicate key!\n");
-            ret = BadAlloc;
-            goto unwind;
-        }
 
         if (!dbus_message_iter_has_next(&subiter))
             MALFORMED_MESSAGE();
@@ -125,20 +111,16 @@ add_device(DBusMessage *message, DBusMessage *reply, DBusError *error)
         if (dbus_message_iter_get_arg_type(&subiter) != DBUS_TYPE_STRING)
             MALFORMED_MESSAGE();
 
-        dbus_message_iter_get_basic(&subiter, &tmp);
+        dbus_message_iter_get_basic(&subiter, &value);
-        if (!tmp)
+        if (!value)
             MALFORMED_MESSAGE();
-        options->value = strdup(tmp);
+
-        if (!options->value) {
+        input_options = input_option_new(input_options, key, value);
-            ErrorF("[config/dbus] couldn't duplicate option!\n");
-            ret = BadAlloc;
-            goto unwind;
-        }
 
         dbus_message_iter_next(&iter);
     }
 
-    ret = NewInputDeviceRequest(options, NULL, &dev);
+    ret = NewInputDeviceRequest(input_options, NULL, &dev);
     if (ret != Success) {
         DebugF("[config/dbus] NewInputDeviceRequest failed\n");
         goto unwind;
@@ -172,13 +154,7 @@ unwind:
         dbus_message_iter_append_basic(&reply_iter, DBUS_TYPE_INT32, &err);
     }
 
-    while (options) {
+    input_option_free_list(&input_options);
-        tmpo = options;
-        options = options->next;
-        free(tmpo->key);
-        free(tmpo->value);
-        free(tmpo);
-    }
 
     return ret;
 }

config/hal.c

@@ -128,7 +128,7 @@ device_added(LibHalContext *hal_ctx, const char *udi)
 {
     char *path = NULL, *driver = NULL, *name = NULL, *config_info = NULL;
     char *hal_tags, *parent;
-    InputOption *options = NULL, *tmpo = NULL;
+    InputOption *input_options = NULL;
     InputAttributes attrs = {0};
     DeviceIntPtr dev = NULL;
     DBusError error;
@@ -205,18 +205,19 @@ device_added(LibHalContext *hal_ctx, const char *udi)
         free(parent);
     }
 
-    if (!add_option(&options, "_source", "server/hal")) {
+    input_options = input_option_new(NULL, "_source", "server/hal");
+    if (!input_options) {
         LogMessage(X_ERROR, "config/hal: couldn't allocate first key/value pair\n");
         goto unwind;
     }
 
     /* most drivers use device.. not path. evdev uses both however, but the
      * path version isn't documented apparently. support both for now. */
-    add_option(&options, "path", path);
+    input_options = input_option_new(input_options, "path", path);
-    add_option(&options, "device", path);
+    input_options = input_option_new(input_options, "device", path);
 
-    add_option(&options, "driver", driver);
+    input_options = input_option_new(input_options, "driver", driver);
-    add_option(&options, "name", name);
+    input_options = input_option_new(input_options, "name", name);
 
     if (asprintf (&config_info, "hal:%s", udi) == -1) {
         config_info = NULL;
@@ -290,7 +291,7 @@ device_added(LibHalContext *hal_ctx, const char *udi)
                     } else
                     {
                         /* all others */
-                        add_option(&options, psi_key + sizeof(LIBHAL_PROP_KEY)-1, tmp_val);
+                        input_options = input_option_new(input_options, psi_key + sizeof(LIBHAL_PROP_KEY)-1, tmp_val);
                         free(tmp_val);
                     }
                 } else
@@ -358,20 +359,20 @@ device_added(LibHalContext *hal_ctx, const char *udi)
 
     /* Now add xkb options */
     if (xkb_opts.layout)
-        add_option(&options, "xkb_layout", xkb_opts.layout);
+        input_options = input_option_new(input_options, "xkb_layout", xkb_opts.layout);
     if (xkb_opts.rules)
-        add_option(&options, "xkb_rules", xkb_opts.rules);
+        input_options = input_option_new(input_options, "xkb_rules", xkb_opts.rules);
     if (xkb_opts.variant)
-        add_option(&options, "xkb_variant", xkb_opts.variant);
+        input_options = input_option_new(input_options, "xkb_variant", xkb_opts.variant);
     if (xkb_opts.model)
-        add_option(&options, "xkb_model", xkb_opts.model);
+        input_options = input_option_new(input_options, "xkb_model", xkb_opts.model);
     if (xkb_opts.options)
-        add_option(&options, "xkb_options", xkb_opts.options);
+        input_options = input_option_new(input_options, "xkb_options", xkb_opts.options);
-    add_option(&options, "config_info", config_info);
+    input_options = input_option_new(input_options, "config_info", config_info);
 
     /* this isn't an error, but how else do you output something that the user can see? */
     LogMessage(X_INFO, "config/hal: Adding input device %s\n", name);
-    if ((rc = NewInputDeviceRequest(options, &attrs, &dev)) != Success) {
+    if ((rc = NewInputDeviceRequest(input_options, &attrs, &dev)) != Success) {
         LogMessage(X_ERROR, "config/hal: NewInputDeviceRequest failed (%d)\n", rc);
         dev = NULL;
         goto unwind;
@@ -384,12 +385,7 @@ unwind:
     free(driver);
     free(name);
     free(config_info);
-    while ((tmpo = options)) {
+    input_option_free_list(&input_options);
-        options = tmpo->next;
-        free(tmpo->key);        /* NULL if dev != NULL */
-        free(tmpo->value);      /* NULL if dev != NULL */
-        free(tmpo);
-    }
 
     free(attrs.product);
     free(attrs.vendor);

config/udev.c

@@ -60,7 +60,7 @@ device_added(struct udev_device *udev_device)
     const char *syspath;
     const char *tags_prop;
     const char *key, *value, *tmp;
-    InputOption *options = NULL, *tmpo;
+    InputOption *input_options;
     InputAttributes attrs = {};
     DeviceIntPtr dev = NULL;
     struct udev_list_entry *set, *entry;
@@ -93,8 +93,9 @@ device_added(struct udev_device *udev_device)
         return;
     }
 
-    if (!add_option(&options, "_source", "server/udev"))
+    input_options = input_option_new(NULL, "_source", "server/udev");
-        goto unwind;
+    if (!input_options)
+        return;
 
     parent = udev_device_get_parent(udev_device);
     if (parent) {
@@ -127,10 +128,9 @@ device_added(struct udev_device *udev_device)
         name = "(unnamed)";
     else
         attrs.product = strdup(name);
-    add_option(&options, "name", name);
+    input_options = input_option_new(input_options, "name", name);
-
+    input_options = input_option_new(input_options, "path", path);
-    add_option(&options, "path", path);
+    input_options = input_option_new(input_options, "device", path);
-    add_option(&options, "device", path);
     if (path)
         attrs.device = strdup(path);
 
@@ -160,15 +160,15 @@ device_added(struct udev_device *udev_device)
             LOG_PROPERTY(path, key, value);
             tmp = key + sizeof(UDEV_XKB_PROP_KEY) - 1;
             if (!strcasecmp(tmp, "rules"))
-                add_option(&options, "xkb_rules", value);
+                input_options = input_option_new(input_options, "xkb_rules", value);
             else if (!strcasecmp(tmp, "layout"))
-                add_option(&options, "xkb_layout", value);
+                input_options = input_option_new(input_options, "xkb_layout", value);
             else if (!strcasecmp(tmp, "variant"))
-                add_option(&options, "xkb_variant", value);
+                input_options = input_option_new(input_options, "xkb_variant", value);
             else if (!strcasecmp(tmp, "model"))
-                add_option(&options, "xkb_model", value);
+                input_options = input_option_new(input_options, "xkb_model", value);
             else if (!strcasecmp(tmp, "options"))
-                add_option(&options, "xkb_options", value);
+                input_options = input_option_new(input_options, "xkb_options", value);
         } else if (!strcmp(key, "ID_VENDOR")) {
             LOG_PROPERTY(path, key, value);
             attrs.vendor = strdup(value);
@@ -193,22 +193,17 @@ device_added(struct udev_device *udev_device)
         }
     }
 
-    add_option(&options, "config_info", config_info);
+    input_options = input_option_new(input_options, "config_info", config_info);
 
     LogMessage(X_INFO, "config/udev: Adding input device %s (%s)\n",
                name, path);
-    rc = NewInputDeviceRequest(options, &attrs, &dev);
+    rc = NewInputDeviceRequest(input_options, &attrs, &dev);
     if (rc != Success)
         goto unwind;
 
  unwind:
     free(config_info);
-    while ((tmpo = options)) {
+    input_option_free_list(&input_options);
-        options = tmpo->next;
-        free(tmpo->key);        /* NULL if dev != NULL */
-        free(tmpo->value);      /* NULL if dev != NULL */
-        free(tmpo);
-    }
 
     free(attrs.usb_id);
     free(attrs.pnp_id);

dix/inpututils.c

@@ -598,3 +598,144 @@ void init_device_event(DeviceEvent *event, DeviceIntPtr dev, Time ms)
     event->deviceid = dev->id;
     event->sourceid = dev->id;
 }
+
+/**
+ * Delete the element with the key from the list, freeing all memory
+ * associated with the element..
+ */
+static void
+input_option_free(InputOption *o)
+{
+    free(o->key);
+    free(o->value);
+    free(o);
+}
+
+/*
+ * Create a new InputOption with the key/value pair provided.
+ * If a list is provided, the new options is added to the list and the list
+ * is returned.
+ *
+ * If a new option is added to a list that already contains that option, the
+ * previous option is overwritten.
+ *
+ * @param list The list to add to.
+ * @param key Option key, will be copied.
+ * @param value Option value, will be copied.
+ *
+ * @return If list is not NULL, the list with the new option added. If list
+ * is NULL, a new option list with one element. On failure, NULL is
+ * returned.
+ */
+InputOption*
+input_option_new(InputOption* list, const char *key, const char *value)
+{
+    InputOption *opt = NULL;
+
+    if (!key)
+        return NULL;
+
+    if (list)
+    {
+        nt_list_for_each_entry(opt, list, next)
+        {
+            if (strcmp(input_option_get_key(opt), key) == 0)
+            {
+                input_option_set_value(opt, value);
+                return list;
+            }
+        }
+    }
+
+    opt = calloc(1, sizeof(InputOption));
+    if (!opt)
+        return NULL;
+
+    nt_list_init(opt, next);
+    input_option_set_key(opt, key);
+    input_option_set_value(opt, value);
+
+    if (list)
+    {
+        nt_list_append(opt, list, InputOption, next);
+        return list;
+    } else
+        return opt;
+}
+
+InputOption*
+input_option_free_element(InputOption *list, const char *key)
+{
+    InputOption *element;
+
+    nt_list_for_each_entry(element, list, next) {
+        if (strcmp(input_option_get_key(element), key) == 0) {
+            nt_list_del(element, list, InputOption, next);
+            input_option_free(element);
+            break;
+        }
+    }
+    return list;
+}
+
+/**
+ * Free the list pointed at by opt.
+ */
+void
+input_option_free_list(InputOption **opt)
+{
+    InputOption *element, *tmp;
+
+    nt_list_for_each_entry_safe(element, tmp, *opt, next) {
+        nt_list_del(element, *opt, InputOption, next);
+        input_option_free(element);
+    }
+    *opt = NULL;
+}
+
+
+/**
+ * Find the InputOption with the given option name.
+ *
+ * @return The InputOption or NULL if not present.
+ */
+InputOption*
+input_option_find(InputOption *list, const char *key)
+{
+    InputOption *element;
+
+    nt_list_for_each_entry(element, list, next) {
+        if (strcmp(input_option_get_key(element), key) == 0)
+            return element;
+    }
+
+    return NULL;
+}
+
+const char*
+input_option_get_key(const InputOption *opt)
+{
+    return opt->key;
+}
+
+const char*
+input_option_get_value(const InputOption *opt)
+{
+    return opt->value;
+}
+
+void
+input_option_set_key(InputOption *opt, const char *key)
+{
+    free(opt->key);
+    if (key)
+        opt->key = strdup(key);
+}
+
+void
+input_option_set_value(InputOption *opt, const char *value)
+{
+    free(opt->value);
+    if (value)
+        opt->value = strdup(value);
+}

hw/xfree86/common/xf86Xinput.c

@@ -879,35 +879,35 @@ NewInputDeviceRequest (InputOption *options, InputAttributes *attrs,
     if (!pInfo)
         return BadAlloc;
 
-    for (option = options; option; option = option->next) {
+    nt_list_for_each_entry(option, options, next) {
-        if (strcasecmp(option->key, "driver") == 0) {
+        if (strcasecmp(input_option_get_key(option), "driver") == 0) {
             if (pInfo->driver) {
                 rval = BadRequest;
                 goto unwind;
             }
-            pInfo->driver = xstrdup(option->value);
+            pInfo->driver = xstrdup(input_option_get_value(option));
             if (!pInfo->driver) {
                 rval = BadAlloc;
                 goto unwind;
             }
         }
 
-        if (strcasecmp(option->key, "name") == 0 ||
+        if (strcasecmp(input_option_get_key(option), "name") == 0 ||
-            strcasecmp(option->key, "identifier") == 0) {
+            strcasecmp(input_option_get_key(option), "identifier") == 0) {
             if (pInfo->name) {
                 rval = BadRequest;
                 goto unwind;
             }
-            pInfo->name = xstrdup(option->value);
+            pInfo->name = xstrdup(input_option_get_value(option));
             if (!pInfo->name) {
                 rval = BadAlloc;
                 goto unwind;
             }
         }
 
-        if (strcmp(option->key, "_source") == 0 &&
+        if (strcmp(input_option_get_key(option), "_source") == 0 &&
-            (strcmp(option->value, "server/hal") == 0 ||
+            (strcmp(input_option_get_value(option), "server/hal") == 0 ||
-             strcmp(option->value, "server/udev") == 0)) {
+             strcmp(input_option_get_value(option), "server/udev") == 0)) {
             is_auto = 1;
             if (!xf86Info.autoAddDevices) {
                 rval = BadMatch;
@@ -916,13 +916,11 @@ NewInputDeviceRequest (InputOption *options, InputAttributes *attrs,
         }
     }
 
-    for (option = options; option; option = option->next) {
+    nt_list_for_each_entry(option, options, next) {
-        /* Steal option key/value strings from the provided list.
+        /* Copy option key/value strings from the provided list */
-         * We need those strings, the InputOption list doesn't. */
+        pInfo->options = xf86AddNewOption(pInfo->options,
-        pInfo->options = xf86addNewOption(pInfo->options,
+                                          input_option_get_key(option),
-                                               option->key, option->value);
+                                          input_option_get_value(option));
-        option->key = NULL;
-        option->value = NULL;
     }
 
     /* Apply InputClass settings */

include/input.h

@@ -56,6 +56,7 @@ SOFTWARE.
 #include "window.h"     /* for WindowPtr */
 #include "xkbrules.h"
 #include "events.h"
+#include "list.h"
 
 #define DEVICE_INIT	0
 #define DEVICE_ON	1
@@ -202,11 +203,7 @@ typedef struct {
 extern _X_EXPORT KeybdCtrl	defaultKeyboardControl;
 extern _X_EXPORT PtrCtrl	defaultPointerControl;
 
-typedef struct _InputOption {
+typedef struct _InputOption InputOption;
-    char                *key;
-    char                *value;
-    struct _InputOption *next;
-} InputOption;
 
 typedef struct _InputAttributes {
     char                *product;
@@ -595,4 +592,14 @@ extern _X_EXPORT void valuator_mask_copy(ValuatorMask *dest,
                                          const ValuatorMask *src);
 extern _X_EXPORT int valuator_mask_get(const ValuatorMask *mask, int valnum);
 
+/* InputOption handling interface */
+extern _X_EXPORT InputOption* input_option_new(InputOption *list, const char *key, const char *value);
+extern _X_EXPORT void input_option_free_list(InputOption **opt);
+extern _X_EXPORT InputOption* input_option_free_element(InputOption *opt, const char *key);
+extern _X_EXPORT InputOption* input_option_find(InputOption *list, const char *key);
+extern _X_EXPORT const char* input_option_get_key(const InputOption *opt);
+extern _X_EXPORT const char* input_option_get_value(const InputOption *opt);
+extern _X_EXPORT void input_option_set_key(InputOption *opt, const char* key);
+extern _X_EXPORT void input_option_set_value(InputOption *opt, const char* value);
+
 #endif /* INPUT_H */

include/inputstr.h

@@ -602,4 +602,11 @@ static inline WindowPtr DeepestSpriteWin(SpritePtr sprite)
     return sprite->spriteTrace[sprite->spriteTraceGood - 1];
 }
 
+struct _InputOption {
+    char                *key;
+    char                *value;
+    struct _InputOption *next;
+};
+
+
 #endif /* INPUTSTRUCT_H */

test/input.c

@@ -1308,6 +1308,95 @@ static void dix_get_master(void)
 }
 
 
+static void input_option_test(void)
+{
+    InputOption *list = NULL;
+    InputOption *opt;
+    const char *val;
+
+    printf("Testing input_option list interface\n");
+
+    list = input_option_new(list, "key", "value");
+    assert(list);
+    opt = input_option_find(list, "key");
+    val = input_option_get_value(opt);
+    assert(strcmp(val, "value") == 0);
+
+    list = input_option_new(list, "2", "v2");
+    opt = input_option_find(list, "key");
+    val = input_option_get_value(opt);
+    assert(strcmp(val, "value") == 0);
+
+    opt = input_option_find(list, "2");
+    val = input_option_get_value(opt);
+    assert(strcmp(val, "v2") == 0);
+
+    list = input_option_new(list, "3", "v3");
+
+    /* search, delete */
+    opt = input_option_find(list, "key");
+    val = input_option_get_value(opt);
+    assert(strcmp(val, "value") == 0);
+    list = input_option_free_element(list, "key");
+    opt = input_option_find(list, "key");
+    assert(opt == NULL);
+
+    opt = input_option_find(list, "2");
+    val = input_option_get_value(opt);
+    assert(strcmp(val, "v2") == 0);
+    list = input_option_free_element(list, "2");
+    opt = input_option_find(list, "2");
+    assert(opt == NULL);
+
+    opt = input_option_find(list, "3");
+    val = input_option_get_value(opt);
+    assert(strcmp(val, "v3") == 0);
+    list = input_option_free_element(list, "3");
+    opt = input_option_find(list, "3");
+    assert(opt == NULL);
+
+    /* list deletion */
+    list = input_option_new(list, "1", "v3");
+    list = input_option_new(list, "2", "v3");
+    list = input_option_new(list, "3", "v3");
+    input_option_free_list(&list);
+
+    assert(list == NULL);
+
+    list = input_option_new(list, "1", "v1");
+    list = input_option_new(list, "2", "v2");
+    list = input_option_new(list, "3", "v3");
+
+    /* value replacement */
+    opt = input_option_find(list, "2");
+    val = input_option_get_value(opt);
+    assert(strcmp(val, "v2") == 0);
+    input_option_set_value(opt, "foo");
+    val = input_option_get_value(opt);
+    assert(strcmp(val, "foo") == 0);
+    opt = input_option_find(list, "2");
+    val = input_option_get_value(opt);
+    assert(strcmp(val, "foo") == 0);
+
+    /* key replacement */
+    input_option_set_key(opt, "bar");
+    val = input_option_get_key(opt);
+    assert(strcmp(val, "bar") == 0);
+    opt = input_option_find(list, "bar");
+    val = input_option_get_value(opt);
+    assert(strcmp(val, "foo") == 0);
+
+    /* value replacement in input_option_new */
+    list = input_option_new(list, "bar", "foobar");
+    opt = input_option_find(list, "bar");
+    val = input_option_get_value(opt);
+    assert(strcmp(val, "foobar") == 0);
+
+    input_option_free_list(&list);
+    assert(list == NULL);
+}
+
+
 int main(int argc, char** argv)
 {
     dix_input_valuator_masks();
@@ -1324,6 +1413,7 @@ int main(int argc, char** argv)
     xi_unregister_handlers();
     dix_valuator_alloc();
     dix_get_master();
+    input_option_test();
 
     return 0;
 }