xserver: Branch 'master' - 7 commits

Keith Packard keithp at kemper.freedesktop.org
Mon Apr 4 11:35:11 PDT 2011


 dix/eventconvert.c          |   27 ++++-----
 hw/xfree86/loader/loadmod.c |    6 ++
 xkb/XKBGAlloc.c             |   64 +++++++++++++++++++--
 xkb/xkbUtils.c              |  129 ++++++++++++--------------------------------
 xkb/xkbgeom.h               |   20 ++++++
 5 files changed, 132 insertions(+), 114 deletions(-)

New commits:
commit a52049de2f846fe984d4db5ac8d2c1826c7b2d0b
Merge: d044d36... 266ea63...
Author: Peter Hutterer <peter.hutterer at who-t.net>
Date:   Mon Apr 4 09:58:53 2011 +1000

    Merge branch 'master' of git://people.freedesktop.org/~herrb/xserver into for-keith

commit d044d3675635f037bf0eb30e47f82460f78227d1
Author: Tiago Vignatti <tiago.vignatti at nokia.com>
Date:   Thu Mar 31 16:26:06 2011 +0300

    xfree86: loader: fix memory leaks in LoaderListDirs
    
    Signed-off-by: Tiago Vignatti <tiago.vignatti at nokia.com>
    Reviewed-by: Peter Hutterer <peter.hutterer at who-t.net>
    Reviewed-by: Nicolas Peninguy <nico at lostgeeks.org>
    Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>

diff --git a/hw/xfree86/loader/loadmod.c b/hw/xfree86/loader/loadmod.c
index eaa99e8..46ce68b 100644
--- a/hw/xfree86/loader/loadmod.c
+++ b/hw/xfree86/loader/loadmod.c
@@ -532,6 +532,7 @@ LoaderListDirs(const char **subdirlist, const char **patternlist)
 				FreePathList(pathlist);
 				FreeSubdirs(subdirs);
 				FreePatterns(patterns);
+				closedir(d);
 				return NULL;
 			    }
 			    listing[n] = malloc(len + 1);
@@ -540,6 +541,7 @@ LoaderListDirs(const char **subdirlist, const char **patternlist)
 				FreePathList(pathlist);
 				FreeSubdirs(subdirs);
 				FreePatterns(patterns);
+				closedir(d);
 				return NULL;
 			    }
 			    strncpy(listing[n], dp->d_name + match[1].rm_so,
@@ -556,6 +558,10 @@ LoaderListDirs(const char **subdirlist, const char **patternlist)
     }
     if (listing)
 	listing[n] = NULL;
+
+    FreePathList(pathlist);
+    FreeSubdirs(subdirs);
+    FreePatterns(patterns);
     return listing;
 }
 
commit 9c4aae2141161e4bf69313a771db91c0acc4cc83
Author: Rami Ylimäki <rami.ylimaki at vincit.fi>
Date:   Wed Mar 30 16:47:31 2011 +0300

    xkb: Prevent leaking of XKB geometry information on copy.
    
    Currently shapes, sections and doodads may leak on copy.
    
    Reviewed-by: Peter Hutterer <peter.hutterer at who-t.net>
    Reviewed-by: Erkki Seppälä <erkki.seppala at vincit.fi>
    Signed-off-by: Rami Ylimäki <rami.ylimaki at vincit.fi>
    Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>

diff --git a/xkb/xkbUtils.c b/xkb/xkbUtils.c
index a80a8d8..cc9aaa7 100644
--- a/xkb/xkbUtils.c
+++ b/xkb/xkbUtils.c
@@ -1541,10 +1541,10 @@ _XkbCopyGeom(XkbDescPtr src, XkbDescPtr dst)
         }
 
         if (src->geom->num_shapes) {
-            tmp = calloc(src->geom->num_shapes, sizeof(XkbShapeRec));
-            if (!tmp)
+            /* Reallocate and clear all items. */
+            if (!XkbGeomRealloc((void **)&dst->geom->shapes, dst->geom->sz_shapes, src->geom->num_shapes,
+                                sizeof(XkbShapeRec), XKB_GEOM_CLEAR_ALL))
                 return FALSE;
-            dst->geom->shapes = tmp;
 
             for (i = 0, sshape = src->geom->shapes, dshape = dst->geom->shapes;
                  i < src->geom->num_shapes;
@@ -1661,7 +1661,6 @@ _XkbCopyGeom(XkbDescPtr src, XkbDescPtr dst)
             }
 
             dst->geom->num_sections = 0;
-            dst->geom->sections = NULL;
         }
 
         if (src->geom->num_sections) {
@@ -1771,7 +1770,6 @@ _XkbCopyGeom(XkbDescPtr src, XkbDescPtr dst)
                 }
             }
             dst->geom->num_doodads = 0;
-            dst->geom->doodads = NULL;
         }
 
         if (src->geom->num_doodads) {
commit 29d63ba175ff1ef1587c390b18ce61c8f1c150f3
Author: Rami Ylimäki <rami.ylimaki at vincit.fi>
Date:   Wed Mar 30 16:47:30 2011 +0300

    xkb: Introduce helper function to handle similar reallocations.
    
    This is preparation for a memory leak fix and doesn't contain any
    functional changes.
    
    Note that two variables are generally used for reallocation and
    clearing of arrays: geom->sz_elems (reallocation) and geom->num_elems
    (clearing). The interface of XkbGeomRealloc is deliberately kept
    simple and it only accepts geom->sz_elems as argument, because that is
    needed to determine whether the array needs to be resized. When the
    array is cleared, we just assume that either geom->sz_elems and
    geom->num_elems are synchronized to be equal or that unused elements
    are cleared whenever geom->num_elems is set to be less than
    geom->sz_elems without reallocation.
    
    Reviewed-by: Erkki Seppälä <erkki.seppala at vincit.fi>
    Signed-off-by: Rami Ylimäki <rami.ylimaki at vincit.fi>
    Reviewed-by: Peter Hutterer <peter.hutterer at who-t.net>
    Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>

diff --git a/xkb/XKBGAlloc.c b/xkb/XKBGAlloc.c
index 65f92fd..dd2b046 100644
--- a/xkb/XKBGAlloc.c
+++ b/xkb/XKBGAlloc.c
@@ -435,6 +435,57 @@ XkbFreeGeometry(XkbGeometryPtr geom,unsigned which,Bool freeMap)
 
 /***====================================================================***/
 
+/**
+ * Resize and clear an XKB geometry item array. The array size may
+ * grow or shrink unlike in _XkbGeomAlloc.
+ *
+ * @param buffer[in,out]  buffer to reallocate and clear
+ * @param szItems[in]     currently allocated item count for "buffer"
+ * @param nrItems[in]     required item count for "buffer"
+ * @param itemSize[in]    size of a single item in "buffer"
+ * @param clearance[in]   items to clear after reallocation
+ *
+ * @see _XkbGeomAlloc
+ *
+ * @return TRUE if reallocation succeeded. Otherwise FALSE is returned
+ *         and contents of "buffer" aren't touched.
+ */
+Bool
+XkbGeomRealloc(void **buffer, int szItems, int nrItems,
+               int itemSize, XkbGeomClearance clearance)
+{
+    void *items;
+    int clearBegin;
+    /* Check validity of arguments. */
+    if (!buffer)
+        return FALSE;
+    items = *buffer;
+    if (!((items && (szItems > 0)) || (!items && !szItems)))
+        return FALSE;
+    /* Check if there is need to resize. */
+    if (nrItems != szItems)
+        if (!(items = realloc(items, nrItems * itemSize)))
+            return FALSE;
+    /* Clear specified items to zero. */
+    switch (clearance)
+    {
+    case XKB_GEOM_CLEAR_EXCESS:
+        clearBegin = szItems;
+        break;
+    case XKB_GEOM_CLEAR_ALL:
+        clearBegin = 0;
+        break;
+    case XKB_GEOM_CLEAR_NONE:
+    default:
+        clearBegin = nrItems;
+        break;
+    }
+    if (items && (clearBegin < nrItems))
+        memset((char *)items + (clearBegin * itemSize), 0, (nrItems - clearBegin) * itemSize);
+    *buffer = items;
+    return TRUE;
+}
+
 static Status
 _XkbGeomAlloc(	void **		old,
 		unsigned short *	num,
@@ -451,18 +502,15 @@ _XkbGeomAlloc(	void **		old,
 	return Success;
 
     *total= (*num)+num_new;
-    if ((*old)!=NULL)
-	 (*old)= realloc((*old),(*total)*sz_elem);
-    else (*old)= calloc((*total),sz_elem);
-    if ((*old)==NULL) {
+
+    if (!XkbGeomRealloc(old, *num, *total, sz_elem, XKB_GEOM_CLEAR_EXCESS))
+    {
+	free(*old);
+	(*old)= NULL;
 	*total= *num= 0;
 	return BadAlloc;
     }
 
-    if (*num>0) {
-	char *tmp= (char *)(*old);
-	memset(&tmp[sz_elem*(*num)], 0, (num_new*sz_elem));
-    }
     return Success;
 }
 
diff --git a/xkb/xkbUtils.c b/xkb/xkbUtils.c
index 3a56bea..a80a8d8 100644
--- a/xkb/xkbUtils.c
+++ b/xkb/xkbUtils.c
@@ -1398,42 +1398,26 @@ _XkbCopyGeom(XkbDescPtr src, XkbDescPtr dst)
 
         /* properties */
         if (src->geom->num_properties) {
-            if (src->geom->num_properties != dst->geom->sz_properties) {
-                /* If we've got more properties in the destination than
-                 * the source, run through and free all the excess ones
-                 * first. */
-                if (src->geom->num_properties < dst->geom->sz_properties) {
-                    for (i = src->geom->num_properties,
-                         dprop = dst->geom->properties + i;
-                         i < dst->geom->num_properties;
-                         i++, dprop++) {
-                        free(dprop->name);
-                        free(dprop->value);
-                    }
+            /* If we've got more properties in the destination than
+             * the source, run through and free all the excess ones
+             * first. */
+            if (src->geom->num_properties < dst->geom->sz_properties) {
+                for (i = src->geom->num_properties, dprop = dst->geom->properties + i;
+                     i < dst->geom->num_properties;
+                     i++, dprop++) {
+                    free(dprop->name);
+                    free(dprop->value);
                 }
-
-                if (dst->geom->sz_properties)
-                    tmp = realloc(dst->geom->properties,
-                                   src->geom->num_properties *
-                                    sizeof(XkbPropertyRec));
-                else
-                    tmp = malloc(src->geom->num_properties *
-                                  sizeof(XkbPropertyRec));
-                if (!tmp)
-                    return FALSE;
-                dst->geom->properties = tmp;
             }
 
+            /* Reallocate and clear all new items if the buffer grows. */
+            if (!XkbGeomRealloc((void **)&dst->geom->properties, dst->geom->sz_properties, src->geom->num_properties,
+                                sizeof(XkbPropertyRec), XKB_GEOM_CLEAR_EXCESS))
+                return FALSE;
             /* We don't set num_properties as we need it to try and avoid
              * too much reallocing. */
             dst->geom->sz_properties = src->geom->num_properties;
 
-            if (dst->geom->sz_properties > dst->geom->num_properties) {
-                memset(dst->geom->properties + dst->geom->num_properties, 0,
-                      (dst->geom->sz_properties - dst->geom->num_properties) *
-                      sizeof(XkbPropertyRec));
-            }
-
             for (i = 0,
                   sprop = src->geom->properties,
                   dprop = dst->geom->properties;
@@ -1482,36 +1466,20 @@ _XkbCopyGeom(XkbDescPtr src, XkbDescPtr dst)
 
         /* colors */
         if (src->geom->num_colors) {
-            if (src->geom->num_colors != dst->geom->sz_colors) {
-                if (src->geom->num_colors < dst->geom->sz_colors) {
-                    for (i = src->geom->num_colors,
-                         dcolor = dst->geom->colors + i;
-                         i < dst->geom->num_colors;
-                         i++, dcolor++) {
-                        free(dcolor->spec);
-                    }
+            if (src->geom->num_colors < dst->geom->sz_colors) {
+                for (i = src->geom->num_colors, dcolor = dst->geom->colors + i;
+                     i < dst->geom->num_colors;
+                     i++, dcolor++) {
+                    free(dcolor->spec);
                 }
-
-                if (dst->geom->sz_colors)
-                    tmp = realloc(dst->geom->colors,
-                                   src->geom->num_colors *
-                                    sizeof(XkbColorRec));
-                else
-                    tmp = malloc(src->geom->num_colors *
-                                  sizeof(XkbColorRec));
-                if (!tmp)
-                    return FALSE;
-                dst->geom->colors = tmp;
             }
 
+            /* Reallocate and clear all new items if the buffer grows. */
+            if (!XkbGeomRealloc((void **)&dst->geom->colors, dst->geom->sz_colors, src->geom->num_colors,
+                                sizeof(XkbColorRec), XKB_GEOM_CLEAR_EXCESS))
+                return FALSE;
             dst->geom->sz_colors = src->geom->num_colors;
 
-            if (dst->geom->sz_colors > dst->geom->num_colors) {
-                memset(dst->geom->colors + dst->geom->num_colors, 0,
-                      (dst->geom->sz_colors - dst->geom->num_colors) *
-                      sizeof(XkbColorRec));
-            }
-
             for (i = 0,
                   scolor = src->geom->colors,
                   dcolor = dst->geom->colors;
@@ -1697,16 +1665,10 @@ _XkbCopyGeom(XkbDescPtr src, XkbDescPtr dst)
         }
 
         if (src->geom->num_sections) {
-            if (dst->geom->sz_sections)
-                tmp = realloc(dst->geom->sections,
-                               src->geom->num_sections *
-                                sizeof(XkbSectionRec));
-            else
-                tmp = malloc(src->geom->num_sections * sizeof(XkbSectionRec));
-            if (!tmp)
+            /* Reallocate and clear all items. */
+            if (!XkbGeomRealloc((void **)&dst->geom->sections, dst->geom->sz_sections, src->geom->num_sections,
+                                sizeof(XkbSectionRec), XKB_GEOM_CLEAR_ALL))
                 return FALSE;
-            memset(tmp, 0, src->geom->num_sections * sizeof(XkbSectionRec));
-            dst->geom->sections = tmp;
             dst->geom->num_sections = src->geom->num_sections;
             dst->geom->sz_sections = src->geom->num_sections;
 
@@ -1813,17 +1775,10 @@ _XkbCopyGeom(XkbDescPtr src, XkbDescPtr dst)
         }
 
         if (src->geom->num_doodads) {
-            if (dst->geom->sz_doodads)
-                tmp = realloc(dst->geom->doodads,
-                               src->geom->num_doodads *
-                                sizeof(XkbDoodadRec));
-            else
-                tmp = malloc(src->geom->num_doodads *
-                              sizeof(XkbDoodadRec));
-            if (!tmp)
+            /* Reallocate and clear all items. */
+            if (!XkbGeomRealloc((void **)&dst->geom->doodads, dst->geom->sz_doodads, src->geom->num_doodads,
+                                sizeof(XkbDoodadRec), XKB_GEOM_CLEAR_ALL))
                 return FALSE;
-            memset(tmp, 0, src->geom->num_doodads * sizeof(XkbDoodadRec));
-            dst->geom->doodads = tmp;
 
             dst->geom->sz_doodads = src->geom->num_doodads;
 
@@ -1860,20 +1815,14 @@ _XkbCopyGeom(XkbDescPtr src, XkbDescPtr dst)
 
         /* key aliases */
         if (src->geom->num_key_aliases) {
-            if (src->geom->num_key_aliases != dst->geom->sz_key_aliases) {
-                if (dst->geom->sz_key_aliases)
-                    tmp = realloc(dst->geom->key_aliases,
-                                   src->geom->num_key_aliases *
-                                    2 * XkbKeyNameLength);
-                else
-                    tmp = malloc(src->geom->num_key_aliases *
-                                  2 * XkbKeyNameLength);
-                if (!tmp)
-                    return FALSE;
-                dst->geom->key_aliases = tmp;
+            /* Reallocate but don't clear any items. There is no need
+             * to clear anything because data is immediately copied
+             * over the whole memory area with memcpy. */
+            if (!XkbGeomRealloc((void **)&dst->geom->key_aliases, dst->geom->sz_key_aliases, src->geom->num_key_aliases,
+                                2 * XkbKeyNameLength, XKB_GEOM_CLEAR_NONE))
+                return FALSE;
 
-                dst->geom->sz_key_aliases = src->geom->num_key_aliases;
-            }
+            dst->geom->sz_key_aliases = src->geom->num_key_aliases;
 
             memcpy(dst->geom->key_aliases, src->geom->key_aliases,
                    src->geom->num_key_aliases * 2 * XkbKeyNameLength);
diff --git a/xkb/xkbgeom.h b/xkb/xkbgeom.h
index fe4da38..d10b956 100644
--- a/xkb/xkbgeom.h
+++ b/xkb/xkbgeom.h
@@ -311,6 +311,17 @@ typedef struct _XkbGeometrySizes {
 	unsigned short	num_key_aliases;
 } XkbGeometrySizesRec,*XkbGeometrySizesPtr;
 
+/**
+ * Specifies which items should be cleared in an XKB geometry array
+ * when the array is reallocated.
+ */
+typedef enum
+{
+    XKB_GEOM_CLEAR_NONE,   /* Don't clear any items, just reallocate.   */
+    XKB_GEOM_CLEAR_EXCESS, /* Clear new extra items after reallocation. */
+    XKB_GEOM_CLEAR_ALL     /* Clear all items after reallocation.       */
+} XkbGeomClearance;
+
 extern	XkbPropertyPtr
 XkbAddGeomProperty(
     XkbGeometryPtr	/* geom */,
@@ -507,6 +518,15 @@ XkbFreeGeometry(
     Bool		/* freeMap */
 );
 
+extern Bool
+XkbGeomRealloc(
+    void **		/* buffer */,
+    int			/* szItems */,
+    int			/* nrItems */,
+    int			/* itemSize */,
+    XkbGeomClearance	/* clearance */
+);
+
 extern Status
 XkbAllocGeomProps(
     XkbGeometryPtr	/* geom */,
commit f40103cee1d591387359f401a5a7c21f4105aeb4
Author: Chase Douglas <chase.douglas at canonical.com>
Date:   Thu Mar 31 11:29:01 2011 -0400

    Don't report old relative values in getValuatorEvents
    
    Relative valuator values should not be reported in any future events. If
    a relative valuator value is not set in an internal event, set the value
    to 0 for XI 1.x valuator events sent over the wire.
    
    Signed-off-by: Chase Douglas <chase.douglas at canonical.com>
    Reviewed-by: Simon Thum <simon.thum at gmx.de>
    Reviewed-by: Peter Hutterer <peter.hutterer at who-t.net>
    Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>

diff --git a/dix/eventconvert.c b/dix/eventconvert.c
index 5fdd357..a5fe0a9 100644
--- a/dix/eventconvert.c
+++ b/dix/eventconvert.c
@@ -408,8 +408,10 @@ getValuatorEvents(DeviceEvent *ev, deviceValuator *xv)
         for (j = 0; j < xv->num_valuators; j++) {
             if (BitIsOn(ev->valuators.mask, xv->first_valuator + j))
                 valuators[j] = ev->valuators.data[xv->first_valuator + j];
-            else
+            else if (dev->valuator->axes[xv->first_valuator + j].mode == Absolute)
                 valuators[j] = dev->valuator->axisVal[xv->first_valuator + j];
+            else
+                valuators[j] = 0;
         }
 
         if (i + 6 < num_valuators)
commit 8199eac443d2c22d313cb23e39d5e607a8cc7f99
Author: Chase Douglas <chase.douglas at canonical.com>
Date:   Mon Mar 28 16:04:48 2011 -0400

    Handle non continuous valuator data in getValuatorEvents
    
    This allows for masked valuators to be handled properly in XI 1.x
    events. Any unset valuators in the device event are set to the last
    known value when transmitted on the wire through XI 1.x valuator events.
    
    Fixes https://bugs.launchpad.net/ubuntu/+source/xorg-server/+bug/736500
    
    Signed-off-by: Chase Douglas <chase.douglas at canonical.com>
    Reviewed-by: Peter Hutterer <peter.hutterer at who-t.net>
    Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>

diff --git a/dix/eventconvert.c b/dix/eventconvert.c
index 07d53e0..5fdd357 100644
--- a/dix/eventconvert.c
+++ b/dix/eventconvert.c
@@ -383,19 +383,18 @@ getValuatorEvents(DeviceEvent *ev, deviceValuator *xv)
     int i;
     int state = 0;
     int first_valuator, num_valuators;
+    DeviceIntPtr dev = NULL;
 
 
     num_valuators = countValuators(ev, &first_valuator);
     if (num_valuators > 0)
     {
-        DeviceIntPtr dev = NULL;
         dixLookupDevice(&dev, ev->deviceid, serverClient, DixUseAccess);
         /* State needs to be assembled BEFORE the device is updated. */
         state = (dev && dev->key) ? XkbStateFieldFromRec(&dev->key->xkbInfo->state) : 0;
         state |= (dev && dev->button) ? (dev->button->state) : 0;
     }
 
-    /* FIXME: non-continuous valuator data in internal events*/
     for (i = 0; i < num_valuators; i += 6, xv++) {
         INT32 *valuators = &xv->valuator0; // Treat all 6 vals as an array
         int j;
@@ -406,8 +405,12 @@ getValuatorEvents(DeviceEvent *ev, deviceValuator *xv)
         xv->deviceid = ev->deviceid;
         xv->device_state = state;
 
-        for (j = 0; j < xv->num_valuators; j++)
-            valuators[j] = ev->valuators.data[xv->first_valuator + j];
+        for (j = 0; j < xv->num_valuators; j++) {
+            if (BitIsOn(ev->valuators.mask, xv->first_valuator + j))
+                valuators[j] = ev->valuators.data[xv->first_valuator + j];
+            else
+                valuators[j] = dev->valuator->axisVal[xv->first_valuator + j];
+        }
 
         if (i + 6 < num_valuators)
             xv->deviceid |= MORE_EVENTS;
commit ac00ab77d5a00cfd198958aa1afaa4c3ccc6d7bc
Author: Chase Douglas <chase.douglas at canonical.com>
Date:   Mon Mar 28 16:04:47 2011 -0400

    Clean up getValuatorEvents using array loop logic
    
    Signed-off-by: Chase Douglas <chase.douglas at canonical.com>
    Reviewed-by: Jamey Sharp <jamey at minilop.net>
    Reviewed-by: Peter Hutterer <peter.hutterer at who-t.net>
    Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>

diff --git a/dix/eventconvert.c b/dix/eventconvert.c
index c9d1994..07d53e0 100644
--- a/dix/eventconvert.c
+++ b/dix/eventconvert.c
@@ -397,25 +397,17 @@ getValuatorEvents(DeviceEvent *ev, deviceValuator *xv)
 
     /* FIXME: non-continuous valuator data in internal events*/
     for (i = 0; i < num_valuators; i += 6, xv++) {
+        INT32 *valuators = &xv->valuator0; // Treat all 6 vals as an array
+        int j;
+
         xv->type = DeviceValuator;
         xv->first_valuator = first_valuator + i;
         xv->num_valuators = ((num_valuators - i) > 6) ? 6 : (num_valuators - i);
         xv->deviceid = ev->deviceid;
         xv->device_state = state;
-        switch (xv->num_valuators) {
-        case 6:
-            xv->valuator5 = ev->valuators.data[xv->first_valuator + 5];
-        case 5:
-            xv->valuator4 = ev->valuators.data[xv->first_valuator + 4];
-        case 4:
-            xv->valuator3 = ev->valuators.data[xv->first_valuator + 3];
-        case 3:
-            xv->valuator2 = ev->valuators.data[xv->first_valuator + 2];
-        case 2:
-            xv->valuator1 = ev->valuators.data[xv->first_valuator + 1];
-        case 1:
-            xv->valuator0 = ev->valuators.data[xv->first_valuator + 0];
-        }
+
+        for (j = 0; j < xv->num_valuators; j++)
+            valuators[j] = ev->valuators.data[xv->first_valuator + j];
 
         if (i + 6 < num_valuators)
             xv->deviceid |= MORE_EVENTS;


More information about the xorg-commit mailing list