From 6fa73194d556765f6a8dfe33c0f609377d5fb41c Mon Sep 17 00:00:00 2001 From: apnadkarni Date: Mon, 15 May 2023 23:08:17 +0000 Subject: Refactor couple more reallocations --- generic/tclCkalloc.c | 141 +++++++++++++++++++++++++++++++++++++++++++++++++++ generic/tclInt.h | 25 ++++----- generic/tclUtil.c | 72 ++++++-------------------- 3 files changed, 168 insertions(+), 70 deletions(-) diff --git a/generic/tclCkalloc.c b/generic/tclCkalloc.c index 09e140a..1539f4f 100644 --- a/generic/tclCkalloc.c +++ b/generic/tclCkalloc.c @@ -1194,6 +1194,147 @@ Tcl_DbCkfree( } /* + *------------------------------------------------------------------------ + * + * TclAttemptOverAlloc -- + * + * Attempts to allocates memory of the requested size plus some more for + * future growth. + * + * Results: + * Pointer to allocated memory block which is at least as large + * as the requested size or NULL if allocation failed. + * + *------------------------------------------------------------------------ + */ +void * +TclAttemptOverAlloc( + Tcl_Size needed, /* Requested size */ + Tcl_Size *allocatedPtr) /* OUTPUT: Actual allocation size is stored + here if non-NULL. Only modified on success */ +{ + void *ptr; + Tcl_Size attempt = TclUpsizeAlloc(0, needed, TCL_SIZE_MAX); + while (attempt > needed) { + ptr = Tcl_AttemptAlloc(attempt); + if (ptr) + break; + attempt = TclUpsizeRetry(needed, attempt); + } + if (ptr == NULL) { + /* Try exact size as a last resort */ + attempt = needed; + ptr = Tcl_AttemptAlloc(attempt); + } + if (ptr && allocatedPtr) { + *allocatedPtr = attempt; + } + return ptr; +} + +/* + *------------------------------------------------------------------------ + * + * TclOverAlloc -- + * + * Allocates memory of the requested size plus some more for future + * growth. + * + * Results: + * Non-NULL pointer to allocated memory block which is at least as large + * as the requested size. + * + * Side effects: + * Panics if memory of at least the requested size could not be + * allocated. + * + *------------------------------------------------------------------------ + */ +void * +TclOverAlloc(Tcl_Size needed, Tcl_Size *allocatedPtr) +{ + void *ptr = TclAttemptOverAlloc(needed, allocatedPtr); + if (ptr == NULL) { + Tcl_Panic("Failed to allocate %" TCL_SIZE_MODIFIER "d bytes.", needed); + } + return ptr; +} + +/* + *------------------------------------------------------------------------ + * + * TclAttemptOverRealloc -- + * + * Attempts to reallocate memory of the requested size plus some more for + * future growth. + * + * Results: + * Pointer to allocated memory block which is at least as large + * as the requested size or NULL if allocation failed. + * + *------------------------------------------------------------------------ + */ +void * +TclAttemptOverRealloc( + Tcl_Size needed, /* Requested size */ + void *oldPtr, /* Pointer to memory block to reallocate */ + Tcl_Size oldSize, /* Old size if known, or 0 if unknown */ + Tcl_Size *allocatedPtr) /* OUTPUT: Actual allocation size is stored + here if non-NULL. Only modified on success */ +{ + void *ptr; + Tcl_Size attempt = TclUpsizeAlloc(oldSize, needed, TCL_SIZE_MAX); + while (attempt > needed) { + ptr = Tcl_AttemptRealloc(oldPtr, attempt); + if (ptr) + break; + attempt = TclUpsizeRetry(needed, attempt); + } + if (ptr == NULL) { + /* Try exact size as a last resort */ + attempt = needed; + ptr = Tcl_AttemptRealloc(oldPtr, attempt); + } + if (ptr && allocatedPtr) { + *allocatedPtr = attempt; + } + return ptr; +} + +/* + *------------------------------------------------------------------------ + * + * TclOverRealloc -- + * + * Reallocates memory of the requested size plus some more for future + * growth. + * + * Results: + * Non-NULL pointer to allocated memory block which is at least as large + * as the requested size. + * + * Side effects: + * Panics if memory of at least the requested size could not be + * allocated. + * + *------------------------------------------------------------------------ + */ +void * +TclOverRealloc( + Tcl_Size needed, /* Requested size */ + void *oldPtr, /* Pointer to memory block to reallocate */ + Tcl_Size oldSize, /* Old size if known, or 0 if unknown */ + Tcl_Size *allocatedPtr) /* OUTPUT: Actual allocation size is stored + here if non-NULL. Only modified on success */ +{ + void *ptr = TclAttemptOverRealloc(needed, oldPtr, oldSize, allocatedPtr); + if (ptr == NULL) { + Tcl_Panic("Failed to reallocate %" TCL_SIZE_MODIFIER "d bytes.", needed); + } + return ptr; +} + +/* *---------------------------------------------------------------------- * * Tcl_InitMemory -- diff --git a/generic/tclInt.h b/generic/tclInt.h index 660c19f..56bef02 100644 --- a/generic/tclInt.h +++ b/generic/tclInt.h @@ -2875,21 +2875,12 @@ typedef struct ProcessGlobalValue { *---------------------------------------------------------------------- * Common functions for growing allocations. Trivial but allows for * experimenting with growth factors without having to change code in - * multiple places. Usage example: + * multiple places. See TclAttemptOverAlloc and TclAttemptOverRealloc for + * usage examples. Best to use those functions if allocating in bytes. + * Direct use of TclUpsizeAlloc / TclResizeAlloc is needed if allocating in other + * units (say Tcl_UniChar), if there is a fixed size header involved or if + * the max limit is something other than TCL_SIZE_MAX. * - * allocated = TclUpsizeAlloc(oldSize, needed, TCL_SIZE_MAX); - * while (allocated > needed) { - * ptr = Tcl_AttemptRealloc(oldPtr, allocated); - * if (ptr) - * break; - * allocated = TclUpsizeRetry(needed, allocated); - * } - * if (ptr == NULL) { - * // Last resort - exact size - * allocated = needed; - * ptr = Tcl_Realloc(oldPtr, allocated); - * } - * ptr now points to an allocation of size 'allocated' *---------------------------------------------------------------------- */ static inline Tcl_Size @@ -2914,6 +2905,12 @@ static inline Tcl_Size TclUpsizeRetry(Tcl_Size needed, Tcl_Size lastAttempt) { return needed; } } +MODULE_SCOPE void *TclOverAlloc(Tcl_Size needed, Tcl_Size *allocatedPtr); +MODULE_SCOPE void *TclAttemptOverAlloc(Tcl_Size needed, Tcl_Size *allocatedPtr); +MODULE_SCOPE void *TclOverRealloc(Tcl_Size needed, void *oldPtr, + Tcl_Size oldSize, Tcl_Size *allocatedPtr); +MODULE_SCOPE void *TclAttemptOverRealloc(Tcl_Size needed, void *oldPtr, + Tcl_Size oldSize, Tcl_Size *allocatedPtr); /* *---------------------------------------------------------------- diff --git a/generic/tclUtil.c b/generic/tclUtil.c index 455b2a9..e8cd164 100644 --- a/generic/tclUtil.c +++ b/generic/tclUtil.c @@ -2613,20 +2613,9 @@ Tcl_DStringAppend( if (newSize > dsPtr->spaceAvl) { - char *newString; - dsPtr->spaceAvl = - TclUpsizeAlloc(dsPtr->spaceAvl, newSize, TCL_SIZE_MAX); if (dsPtr->string == dsPtr->staticSpace) { - while (dsPtr->spaceAvl > newSize) { - newString = (char *)Tcl_AttemptAlloc(dsPtr->spaceAvl); - if (newString) - break; - dsPtr->spaceAvl = TclUpsizeRetry(newSize, dsPtr->spaceAvl); - } - if (newString == NULL) { - dsPtr->spaceAvl = newSize; - newString = (char *) Tcl_Alloc(dsPtr->spaceAvl); - } + char *newString; + newString = (char *) TclOverAlloc(newSize, &dsPtr->spaceAvl); memcpy(newString, dsPtr->string, dsPtr->length); dsPtr->string = newString; } else { @@ -2638,19 +2627,8 @@ Tcl_DStringAppend( /* Source string is within this DString. Note offset */ offset = bytes - dsPtr->string; } - - while (dsPtr->spaceAvl > newSize) { - newString = (char *)Tcl_AttemptRealloc(dsPtr->string, dsPtr->spaceAvl); - if (newString) - break; - dsPtr->spaceAvl = TclUpsizeRetry(newSize, dsPtr->spaceAvl); - } - if (newString == NULL) { - dsPtr->spaceAvl = newSize; - newString = (char *) Tcl_Realloc(dsPtr->string, dsPtr->spaceAvl); - } - - dsPtr->string = newString; + dsPtr->string = (char *) TclOverRealloc( + newSize, dsPtr->string, dsPtr->spaceAvl, &dsPtr->spaceAvl); if (offset >= 0) { bytes = dsPtr->string + offset; } @@ -2766,20 +2744,9 @@ Tcl_DStringAppendElement( */ newSize += 1; /* For terminating nul */ if (newSize > dsPtr->spaceAvl) { - char *newString; - dsPtr->spaceAvl = - TclUpsizeAlloc(dsPtr->spaceAvl, newSize, TCL_SIZE_MAX); if (dsPtr->string == dsPtr->staticSpace) { - while (dsPtr->spaceAvl > newSize) { - newString = (char *)Tcl_AttemptAlloc(dsPtr->spaceAvl); - if (newString) - break; - dsPtr->spaceAvl = TclUpsizeRetry(newSize, dsPtr->spaceAvl); - } - if (newString == NULL) { - dsPtr->spaceAvl = newSize; - newString = (char *) Tcl_Alloc(dsPtr->spaceAvl); - } + char *newString; + newString = (char *) TclOverAlloc(newSize, &dsPtr->spaceAvl); memcpy(newString, dsPtr->string, dsPtr->length); dsPtr->string = newString; } else { @@ -2791,18 +2758,8 @@ Tcl_DStringAppendElement( /* Source string is within this DString. Note offset */ offset = element - dsPtr->string; } - while (dsPtr->spaceAvl > newSize) { - newString = (char *)Tcl_AttemptRealloc(dsPtr->string, dsPtr->spaceAvl); - if (newString) - break; - dsPtr->spaceAvl = TclUpsizeRetry(newSize, dsPtr->spaceAvl); - } - if (newString == NULL) { - dsPtr->spaceAvl = newSize; - newString = (char *) Tcl_Realloc(dsPtr->string, dsPtr->spaceAvl); - } - - dsPtr->string = newString; + dsPtr->string = (char *) TclOverRealloc( + newSize, dsPtr->string, dsPtr->spaceAvl, &dsPtr->spaceAvl); if (offset >= 0) { element = dsPtr->string + offset; } @@ -2861,13 +2818,16 @@ Tcl_DStringSetLength( * would be wasteful to overallocate that buffer, so we just allocate * enough for the requested size plus the trailing null byte. In the * second case, we are growing the buffer incrementally, so we need - * behavior similar to Tcl_DStringAppend. The requested length will - * usually be a small delta above the current spaceAvl, so we'll end - * up doubling the old size. This won't grow the buffer quite as - * quickly, but it should be close enough. + * behavior similar to Tcl_DStringAppend. + * TODO - the above makes no sense to me. How does the code below + * translate into distinguishing the two cases above? IMO, if caller + * specifically sets the length, there is no cause for overallocation. */ - newsize = dsPtr->spaceAvl * 2; + if (length >= TCL_SIZE_MAX) { + Tcl_Panic("Tcl_Concat: max size of Tcl value exceeded"); + } + newsize = TclUpsizeAlloc(dsPtr->spaceAvl, length + 1, TCL_SIZE_MAX); if (length < newsize) { dsPtr->spaceAvl = newsize; } else { -- cgit v0.12