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.sepp...@vincit.fi> Signed-off-by: Rami Ylimäki <rami.ylim...@vincit.fi> --- xkb/XKBGAlloc.c | 64 +++++++++++++++++++++++++---- xkb/xkbUtils.c | 121 ++++++++++++++++--------------------------------------- xkb/xkbgeom.h | 20 +++++++++ 3 files changed, 111 insertions(+), 94 deletions(-) diff --git a/xkb/XKBGAlloc.c b/xkb/XKBGAlloc.c index d1adea3..dbe0069 100644 --- a/xkb/XKBGAlloc.c +++ b/xkb/XKBGAlloc.c @@ -445,6 +445,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, @@ -461,18 +512,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 14dc784..7fea194 100644 --- a/xkb/xkbUtils.c +++ b/xkb/xkbUtils.c @@ -1395,42 +1395,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; @@ -1479,36 +1463,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; @@ -1694,16 +1662,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; @@ -1810,17 +1772,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; @@ -1857,20 +1812,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 */, -- 1.6.3.3 _______________________________________________ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel