On Sat, May 08, 2010 at 10:51:57PM +0200, Julien Cristau wrote:
> On Sat, May  8, 2010 at 09:06:26 -0700, Dan Nicholson wrote:
> 
> > > Can we make the stuff in InputAttributes const instead?
> > 
> > No, because the strings from hal are not const. I have a patch to make
> > config/udev dup the strings because:
> > 
> > 1. udevd might die
> > 2. the device might get immediately removed
> > 
> > In either case, you're holding a pointer into udev's database that
> > might go away at any time. Doesn't seem like a situation we'd want to
> > be in.
> > 
> I'm holding a pointer to a string that libudev gave me, and that's
> hanging off an udev_device that I'm holding a reference on, until after
> NIDR returns.  I'm not sure how that string could go away?

Yeah, clearly libudev has it's own copies of the strings that it will
keep until we unref the udev_device. So, here's a patch that makes the
InputAttributes members const. Like Keith, I couldn't figure out how to
handle char **tags without an explicit cast.

Dan

>From ea0eff801809dbc086f3ff3e821af5c423b33eb7 Mon Sep 17 00:00:00 2001
From: Dan Nicholson <[email protected]>
Date: Tue, 11 May 2010 10:20:04 -0700
Subject: [PATCH] config: Declare InputAttributes pointers as const

These values are really immutable once they're filled in by the config
backend. Kills a couple warnings in udev where libudev returns const
char * properties that were being assigned to InputAttributes.

Signed-off-by: Dan Nicholson <[email protected]>
---
 I had a change of heart, so here's a patch to make the InputAttributes
 members const.

 config/hal.c    |   12 ++++++------
 config/udev.c   |    8 ++++++--
 include/input.h |    8 ++++----
 3 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/config/hal.c b/config/hal.c
index d3daa84..9b360ec 100644
--- a/config/hal.c
+++ b/config/hal.c
@@ -155,16 +155,18 @@ device_added(LibHalContext *hal_ctx, const char *udi)
         LogMessage(X_WARNING,"config/hal: no driver or path specified for 
%s\n", udi);
         goto unwind;
     }
-    attrs.device = xstrdup(path);
+    attrs.device = path;
 
     name = get_prop_string(hal_ctx, udi, "info.product");
     if (!name)
         name = xstrdup("(unnamed)");
     else
-        attrs.product = xstrdup(name);
+        attrs.product = name;
 
     attrs.vendor = get_prop_string(hal_ctx, udi, "info.vendor");
-    attrs.tags = xstrtokenize(get_prop_string(hal_ctx, udi, "input.tags"), 
",");
+    attrs.tags = (const char **)xstrtokenize(get_prop_string(hal_ctx, udi,
+                                                             "input.tags"),
+                                             ",");
 
     if (libhal_device_query_capability(hal_ctx, udi, "input.keys", NULL))
         attrs.flags |= ATTR_KEYBOARD;
@@ -389,11 +391,9 @@ unwind:
         xfree(tmpo);
     }
 
-    xfree(attrs.product);
     xfree(attrs.vendor);
-    xfree(attrs.device);
     if (attrs.tags) {
-        char **tag = attrs.tags;
+        const char **tag = attrs.tags;
         while (*tag) {
             xfree(*tag);
             tag++;
diff --git a/config/udev.c b/config/udev.c
index 452fb5a..ea981fa 100644
--- a/config/udev.c
+++ b/config/udev.c
@@ -87,7 +87,11 @@ device_added(struct udev_device *udev_device)
     add_option(&options, "path", path);
     add_option(&options, "device", path);
     attrs.device = path;
-    attrs.tags = xstrtokenize(udev_device_get_property_value(udev_device, 
"ID_INPUT.tags"), ",");
+
+    attrs.tags = (const char **)
+        xstrtokenize(udev_device_get_property_value(udev_device,
+                                                    "ID_INPUT.tags"),
+                     ",");
 
     config_info = Xprintf("udev:%s", syspath);
     if (!config_info)
@@ -155,7 +159,7 @@ device_added(struct udev_device *udev_device)
     }
 
     if (attrs.tags) {
-        char **tag = attrs.tags;
+        const char **tag = attrs.tags;
         while (*tag) {
             xfree(*tag);
             tag++;
diff --git a/include/input.h b/include/input.h
index 8561308..0dc2423 100644
--- a/include/input.h
+++ b/include/input.h
@@ -212,10 +212,10 @@ typedef struct _InputOption {
 } InputOption;
 
 typedef struct _InputAttributes {
-    char                *product;
-    char                *vendor;
-    char                *device;
-    char                **tags; /* null-terminated */
+    const char          *product;
+    const char          *vendor;
+    const char          *device;
+    const char          **tags; /* null-terminated */
     uint32_t            flags;
 } InputAttributes;
 
-- 
1.6.6.1
_______________________________________________
[email protected]: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Reply via email to