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@vincit.fi>
Signed-off-by: Rami Ylimäki <rami.ylimaki@vincit.fi>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>

3 files changed, 111 insertions, 94 deletions

diff --git a/xkb/XKBGAlloc.c b/xkb/XKBGAlloc.c
index 65f92fdba..dd2b04696 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 3a56bea4c..a80a8d8fd 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 fe4da3806..d10b956a6 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 */,