[PATCH xserver 1/2] xkb: Introduce helper function to handle similar reallocations.

Rami Ylimäki rami.ylimaki at vincit.fi
Thu Mar 17 01:40:00 PDT 2011


This is preparation for a memory leak fix and doesn't contain any
functional changes.

Reviewed-by: Erkki Seppälä <erkki.seppala at vincit.fi>
Signed-off-by: Rami Ylimäki <rami.ylimaki at vincit.fi>
---
 xkb/xkbUtils.c |  153 ++++++++++++++++++++++++++------------------------------
 1 files changed, 71 insertions(+), 82 deletions(-)

diff --git a/xkb/xkbUtils.c b/xkb/xkbUtils.c
index 3a56bea..f4c8a6e 100644
--- a/xkb/xkbUtils.c
+++ b/xkb/xkbUtils.c
@@ -1375,6 +1375,38 @@ _XkbCopyCompat(XkbDescPtr src, XkbDescPtr dst)
     return TRUE;
 }
 
+/**
+ * Resize and clear an XKB geometry item array.
+ *
+ * @param items[in/out]   array to reallocate and clear
+ * @param szItems[in]     currently allocated item count for "items"
+ * @param nrItems[in]     required item count for "items"
+ * @param clearBegin[in]  first item to clear after allocation
+ * @param clearEnd[in]    one past last item to clear after allocation
+ * @param itemSize[in]    size of a single item in "items"
+ *
+ * @see _XkbGeomAlloc, which is very similar
+ *
+ * @return reallocated array
+ */
+static void *
+_XkbGeomRealloc(void *items, int szItems, int nrItems,
+                int clearBegin, int clearEnd, int itemSize)
+{
+    /* Check if there is need to resize. */
+    if (nrItems != szItems)
+    {
+        assert((items && (szItems > 0)) || (!items && !szItems));
+        items = realloc(items, nrItems * itemSize);
+    }
+    /* Clear specified items to zero. */
+    clearEnd = (clearEnd < nrItems) ? clearEnd : nrItems;
+    if (items && (clearBegin < clearEnd))
+        memset((char *)items + (clearBegin * itemSize), 0, (clearEnd - clearBegin) * itemSize);
+
+    return items;
+}
+
 static Bool
 _XkbCopyGeom(XkbDescPtr src, XkbDescPtr dst)
 {
@@ -1398,42 +1430,28 @@ _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. */
+            tmp = _XkbGeomRealloc(dst->geom->properties, dst->geom->sz_properties, src->geom->num_properties,
+                                  dst->geom->num_properties, src->geom->num_properties, sizeof(XkbPropertyRec));
+            if (!tmp)
+                return FALSE;
+            dst->geom->properties = tmp;
             /* 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 +1500,22 @@ _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. */
+            tmp = _XkbGeomRealloc(dst->geom->colors, dst->geom->sz_colors, src->geom->num_colors,
+                                  dst->geom->num_colors, src->geom->num_colors, sizeof(XkbColorRec));
+            if (!tmp)
+                return FALSE;
+            dst->geom->colors = tmp;
             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,15 +1701,11 @@ _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));
+            /* Reallocate and clear all items. */
+            tmp = _XkbGeomRealloc(dst->geom->sections, dst->geom->sz_sections, src->geom->num_sections,
+                                  0, src->geom->num_sections, sizeof(XkbSectionRec));
             if (!tmp)
                 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,16 +1813,11 @@ _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));
+            /* Reallocate and clear all items. */
+            tmp = _XkbGeomRealloc(dst->geom->doodads, dst->geom->sz_doodads, src->geom->num_doodads,
+                                  0, src->geom->num_doodads, sizeof(XkbDoodadRec));
             if (!tmp)
                 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 +1855,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. */
+            tmp = _XkbGeomRealloc(dst->geom->key_aliases, dst->geom->sz_key_aliases, src->geom->num_key_aliases,
+                                  src->geom->num_key_aliases, src->geom->num_key_aliases, 2 * XkbKeyNameLength);
+            if (!tmp)
+                return FALSE;
+            dst->geom->key_aliases = tmp;
 
-                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);
-- 
1.6.3.3



More information about the xorg-devel mailing list