From 64eaafad72c1c1c0a76fb1adffb7200f79debb04 Mon Sep 17 00:00:00 2001 From: sebres Date: Tue, 13 Mar 2018 18:52:45 +0000 Subject: remove unexpected panic by TclTrimLeft/TclTrimRight, handling rewritten: - instead of check the NTS-byte after string (may cause segfault by access violation if out of range), it checks now the end-character is well-formed utf8-sequence; - both work well now on non-NTS strings (without null character at end); - additionally fixed wrong offsets (trim-length if last char malformed in case of out of range); new function TclTrim introduced (as combination of TclTrimLeft/TclTrimRight). --- generic/tclCmdMZ.c | 3 +- generic/tclInt.h | 2 + generic/tclUtil.c | 228 ++++++++++++++++++++++++++++++++++++++++------------- 3 files changed, 176 insertions(+), 57 deletions(-) diff --git a/generic/tclCmdMZ.c b/generic/tclCmdMZ.c index 30586b1..b20fffb 100644 --- a/generic/tclCmdMZ.c +++ b/generic/tclCmdMZ.c @@ -3127,8 +3127,7 @@ StringTrimCmd( } string1 = TclGetStringFromObj(objv[1], &length1); - triml = TclTrimLeft(string1, length1, string2, length2); - trimr = TclTrimRight(string1 + triml, length1 - triml, string2, length2); + triml = TclTrim(string1, length1, string2, length2, &trimr); Tcl_SetObjResult(interp, Tcl_NewStringObj(string1 + triml, length1 - triml - trimr)); diff --git a/generic/tclInt.h b/generic/tclInt.h index 25bec6a..9d60cbc 100644 --- a/generic/tclInt.h +++ b/generic/tclInt.h @@ -2747,6 +2747,8 @@ MODULE_SCOPE int TclSubstTokens(Tcl_Interp *interp, Tcl_Token *tokenPtr, int *clNextOuter, CONST char *outerScript); MODULE_SCOPE void TclTransferResult(Tcl_Interp *sourceInterp, int result, Tcl_Interp *targetInterp); +MODULE_SCOPE int TclTrim(const char *bytes, int numBytes, + const char *trim, int numTrim, int *trimRight); MODULE_SCOPE int TclTrimLeft(const char *bytes, int numBytes, const char *trim, int numTrim); MODULE_SCOPE int TclTrimRight(const char *bytes, int numBytes, diff --git a/generic/tclUtil.c b/generic/tclUtil.c index d782ea1..96ab96d 100644 --- a/generic/tclUtil.c +++ b/generic/tclUtil.c @@ -1499,9 +1499,46 @@ Tcl_Backslash( /* *---------------------------------------------------------------------- * + * TclUtfWellFormedEnd -- + * Checks the end of utf string is malformed, if yes - wraps bytes + * to the given buffer (as well-formed NTS string). + * + * Results: + * The bytes with well-formed end of the string. + * + * Side effects: + * Buffer (DString) may be allocated, so must be released. + * + *---------------------------------------------------------------------- + */ + +static inline const char* +TclUtfWellFormedEnd( + Tcl_DString *buffer, /* Buffer used to hold well-formed string. */ + CONST char *bytes, /* Pointer to the beginning of the string. */ + int length) /* Length of the string. */ +{ + CONST char *l = bytes + length; + CONST char *p = Tcl_UtfPrev(l, bytes); + + if (Tcl_UtfCharComplete(p, l - p)) { + buffer->string = buffer->staticSpace; + return bytes; + } + /* + * Malformed utf-8 end, be sure we've NTS to safe compare of end-character, + * avoid segfault by access violation out of range. + */ + Tcl_DStringInit(buffer); + Tcl_DStringAppend(buffer, bytes, length); + return Tcl_DStringValue(buffer); +} +/* + *---------------------------------------------------------------------- + * * TclTrimRight -- - * Takes two counted strings in the Tcl encoding which must both be - * null terminated. Conceptually trims from the right side of the + * Takes two counted strings in the Tcl encoding. Conceptually + * finds the sub string (offset) to trim from the right side of the * first string all characters found in the second string. * * Results: @@ -1513,8 +1550,8 @@ Tcl_Backslash( *---------------------------------------------------------------------- */ -int -TclTrimRight( +static inline int +TrimRight( const char *bytes, /* String to be trimmed... */ int numBytes, /* ...and its length in bytes */ const char *trim, /* String of trim characters... */ @@ -1523,15 +1560,6 @@ TclTrimRight( const char *p = bytes + numBytes; int pInc; - if ((bytes[numBytes] != '\0') || (trim[numTrim] != '\0')) { - Tcl_Panic("TclTrimRight works only on null-terminated strings"); - } - - /* Empty strings -> nothing to do */ - if ((numBytes == 0) || (numTrim == 0)) { - return 0; - } - /* Outer loop: iterate over string to be trimmed */ do { Tcl_UniChar ch1; @@ -1563,13 +1591,42 @@ TclTrimRight( return numBytes - (p - bytes); } + +int +TclTrimRight( + const char *bytes, /* String to be trimmed... */ + int numBytes, /* ...and its length in bytes */ + const char *trim, /* String of trim characters... */ + int numTrim) /* ...and its length in bytes */ +{ + int res; + Tcl_DString bytesBuf, trimBuf; + + /* Empty strings -> nothing to do */ + if ((numBytes == 0) || (numTrim == 0)) { + return 0; + } + + bytes = TclUtfWellFormedEnd(&bytesBuf, bytes, numBytes); + trim = TclUtfWellFormedEnd(&trimBuf, trim, numTrim); + + res = TrimRight(bytes, numBytes, trim, numTrim); + if (res > numBytes) { + res = numBytes; + } + + Tcl_DStringFree(&bytesBuf); + Tcl_DStringFree(&trimBuf); + + return res; +} /* *---------------------------------------------------------------------- * * TclTrimLeft -- - * Takes two counted strings in the Tcl encoding which must both be - * null terminated. Conceptually trims from the left side of the + * Takes two counted strings in the Tcl encoding. Conceptually + * finds the sub string (offset) to trim from the left side of the * first string all characters found in the second string. * * Results: @@ -1581,8 +1638,8 @@ TclTrimRight( *---------------------------------------------------------------------- */ -int -TclTrimLeft( +static inline int +TrimLeft( const char *bytes, /* String to be trimmed... */ int numBytes, /* ...and its length in bytes */ const char *trim, /* String of trim characters... */ @@ -1590,15 +1647,6 @@ TclTrimLeft( { const char *p = bytes; - if ((bytes[numBytes] != '\0') || (trim[numTrim] != '\0')) { - Tcl_Panic("TclTrimLeft works only on null-terminated strings"); - } - - /* Empty strings -> nothing to do */ - if ((numBytes == 0) || (numTrim == 0)) { - return 0; - } - /* Outer loop: iterate over string to be trimmed */ do { Tcl_UniChar ch1; @@ -1626,10 +1674,94 @@ TclTrimLeft( p += pInc; numBytes -= pInc; - } while (numBytes); + } while (numBytes > 0); return p - bytes; } + +int +TclTrimLeft( + const char *bytes, /* String to be trimmed... */ + int numBytes, /* ...and its length in bytes */ + const char *trim, /* String of trim characters... */ + int numTrim) /* ...and its length in bytes */ +{ + int res; + Tcl_DString bytesBuf, trimBuf; + + /* Empty strings -> nothing to do */ + if ((numBytes == 0) || (numTrim == 0)) { + return 0; + } + + bytes = TclUtfWellFormedEnd(&bytesBuf, bytes, numBytes); + trim = TclUtfWellFormedEnd(&trimBuf, trim, numTrim); + + res = TrimLeft(bytes, numBytes, trim, numTrim); + if (res > numBytes) { + res = numBytes; + } + + Tcl_DStringFree(&bytesBuf); + Tcl_DStringFree(&trimBuf); + + return res; +} + +/* + *---------------------------------------------------------------------- + * + * TclTrim -- + * Finds the sub string (offset) to trim from both sides of the + * first string all characters found in the second string. + * + * Results: + * The number of bytes to be removed from the start of the string + * + * Side effects: + * None. + * + *---------------------------------------------------------------------- + */ + +int +TclTrim( + const char *bytes, /* String to be trimmed... */ + int numBytes, /* ...and its length in bytes */ + const char *trim, /* String of trim characters... */ + int numTrim, /* ...and its length in bytes */ + int *trimRight) /* Offset from the end of the string. */ +{ + int trimLeft; + Tcl_DString bytesBuf, trimBuf; + + /* Empty strings -> nothing to do */ + if ((numBytes == 0) || (numTrim == 0)) { + return 0; + } + + bytes = TclUtfWellFormedEnd(&bytesBuf, bytes, numBytes); + trim = TclUtfWellFormedEnd(&trimBuf, trim, numTrim); + + trimLeft = TrimLeft(bytes, numBytes, trim, numTrim); + if (trimLeft > numBytes) { + trimLeft = numBytes; + } + numBytes -= trimLeft; + *trimRight = 0; + if (numBytes) { + bytes += trimLeft; + *trimRight = TrimRight(bytes, numBytes, trim, numTrim); + if (*trimRight > numBytes) { + *trimRight = numBytes; + } + } + + Tcl_DStringFree(&bytesBuf); + Tcl_DStringFree(&trimBuf); + + return trimLeft; +} /* *---------------------------------------------------------------------- @@ -1687,25 +1819,18 @@ Tcl_Concat( result = (char *) ckalloc((unsigned) (bytesNeeded + argc)); for (p = result, i = 0; i < argc; i++) { - int trim, elemLength; + int triml, trimr, elemLength; const char *element; element = argv[i]; elemLength = strlen(argv[i]); - /* Trim away the leading whitespace */ - trim = TclTrimLeft(element, elemLength, CONCAT_WS, CONCAT_WS_SIZE); - element += trim; - elemLength -= trim; - - /* - * Trim away the trailing whitespace. Do not permit trimming - * to expose a final backslash character. - */ - - trim = TclTrimRight(element, elemLength, CONCAT_WS, CONCAT_WS_SIZE); - trim -= trim && (element[elemLength - trim - 1] == '\\'); - elemLength -= trim; + /* Trim away the leading/trailing whitespace. */ + triml = TclTrim(element, elemLength, CONCAT_WS, CONCAT_WS_SIZE, &trimr); + element += triml; + elemLength -= triml + trimr; + /* Do not permit trimming to expose a final backslash character. */ + elemLength += trimr && (element[elemLength - 1] == '\\'); /* If we're left with empty element after trimming, do nothing */ if (elemLength == 0) { @@ -1832,23 +1957,16 @@ Tcl_ConcatObj( Tcl_SetObjLength(resPtr, 0); for (i = 0; i < objc; i++) { - int trim; + int triml, trimr; element = TclGetStringFromObj(objv[i], &elemLength); - /* Trim away the leading whitespace */ - trim = TclTrimLeft(element, elemLength, CONCAT_WS, CONCAT_WS_SIZE); - element += trim; - elemLength -= trim; - - /* - * Trim away the trailing whitespace. Do not permit trimming - * to expose a final backslash character. - */ - - trim = TclTrimRight(element, elemLength, CONCAT_WS, CONCAT_WS_SIZE); - trim -= trim && (element[elemLength - trim - 1] == '\\'); - elemLength -= trim; + /* Trim away the leading/trailing whitespace. */ + triml = TclTrim(element, elemLength, CONCAT_WS, CONCAT_WS_SIZE, &trimr); + element += triml; + elemLength -= triml + trimr; + /* Do not permit trimming to expose a final backslash character. */ + elemLength += trimr && (element[elemLength - 1] == '\\'); /* If we're left with empty element after trimming, do nothing */ if (elemLength == 0) { -- cgit v0.12 From 90f38b1023a10f8a5518bdcd04cb4e377d709fe9 Mon Sep 17 00:00:00 2001 From: pooryorick Date: Tue, 13 Mar 2018 20:41:23 +0000 Subject: Audit and correct Object reference counting issues. --- generic/tclOO.c | 196 ++++++++++++++++++++++++++-------------------- generic/tclOODefineCmds.c | 10 +-- tests/oo.test | 8 +- 3 files changed, 123 insertions(+), 91 deletions(-) diff --git a/generic/tclOO.c b/generic/tclOO.c index 587e46d..a129a52 100644 --- a/generic/tclOO.c +++ b/generic/tclOO.c @@ -397,28 +397,39 @@ InitFoundation( fPtr->objectCls = AllocClass(interp, AllocObject(interp, "object", (Namespace *)fPtr->ooNs, NULL)); + /* Corresponding TclOODecrRefCount in KillFoudation */ + AddRef(fPtr->objectCls->thisPtr); + + /* This is why it is unnecessary in this routine to replace the + * incremented reference count of fPtr->objectCls that was swallowed by + * fakeObject. */ + fPtr->objectCls->superclasses.num = 0; + ckfree(fPtr->objectCls->superclasses.list); + fPtr->objectCls->superclasses.list = NULL; + + /* special initialization for the primordial objects */ + fPtr->objectCls->thisPtr->flags |= ROOT_OBJECT; + fPtr->objectCls->flags |= ROOT_OBJECT; + fPtr->classCls = AllocClass(interp, AllocObject(interp, "class", (Namespace *)fPtr->ooNs, NULL)); + /* Corresponding TclOODecrRefCount in KillFoudation */ + AddRef(fPtr->classCls->thisPtr); + + /* + * Increment reference counts for each reference because these + * relationships can be dynamically changed. + * + * Corresponding TclOODecrRefCount for all incremented refcounts is in + * KillFoundation. + */ /* Rewire bootstrapped objects. */ fPtr->objectCls->thisPtr->selfCls = fPtr->classCls; - fPtr->classCls->thisPtr->selfCls = fPtr->classCls; - - AddRef(fPtr->objectCls->thisPtr); - AddRef(fPtr->classCls->thisPtr); - AddRef(fPtr->classCls->thisPtr->selfCls->thisPtr); AddRef(fPtr->objectCls->thisPtr->selfCls->thisPtr); - /* special initialization for the primordial objects */ - fPtr->objectCls->thisPtr->flags |= ROOT_OBJECT; - fPtr->objectCls->flags |= ROOT_OBJECT; - - /* This is why it is unnecessary in this routine to make up for the - * incremented reference count of fPtr->objectCls that was sallwed by - * fakeObject. */ - fPtr->objectCls->superclasses.num = 0; - ckfree(fPtr->objectCls->superclasses.list); - fPtr->objectCls->superclasses.list = NULL; + fPtr->classCls->thisPtr->selfCls = fPtr->classCls; + AddRef(fPtr->classCls->thisPtr->selfCls->thisPtr); fPtr->classCls->thisPtr->flags |= ROOT_CLASS; fPtr->classCls->flags |= ROOT_CLASS; @@ -552,20 +563,20 @@ KillFoundation( { Foundation *fPtr = GetFoundation(interp); - /* - * Crude mechanism to avoid leaking the Object struct of the - * foundation components oo::object and oo::class - * - * Should probably be replaced with something more elegantly designed. - */ - while (TclOODecrRefCount(fPtr->objectCls->thisPtr) == 0) {}; - while (TclOODecrRefCount(fPtr->classCls->thisPtr) == 0) {}; - TclDecrRefCount(fPtr->unknownMethodNameObj); TclDecrRefCount(fPtr->constructorName); TclDecrRefCount(fPtr->destructorName); TclDecrRefCount(fPtr->clonedName); TclDecrRefCount(fPtr->defineName); + if (fPtr->objectCls->thisPtr->selfCls != NULL) { + TclOODecrRefCount(fPtr->objectCls->thisPtr->selfCls->thisPtr); + } + if (fPtr->classCls->thisPtr->selfCls != NULL) { + TclOODecrRefCount(fPtr->classCls->thisPtr->selfCls->thisPtr); + } + TclOODecrRefCount(fPtr->objectCls->thisPtr); + TclOODecrRefCount(fPtr->classCls->thisPtr); + ckfree(fPtr); } @@ -649,6 +660,8 @@ AllocObject( Tcl_ResetResult(interp); } + ((Namespace *)oPtr->namespacePtr)->refCount++; + /* * Make the namespace know about the helper commands. This grants access * to the [self] and [next] commands. @@ -820,10 +833,9 @@ ObjectRenamedTrace( /* * ---------------------------------------------------------------------- * - * ReleaseClassContents -- + * DeleteDescendants -- * - * Tear down the special class data structure, including deleting all - * dependent classes and objects. + * Delete all descendants of a particular class. * * ---------------------------------------------------------------------- */ @@ -835,50 +847,79 @@ DeleteDescendants( { Class *clsPtr = oPtr->classPtr, *subclassPtr, *mixinSubclassPtr; Object *instancePtr; - int i; /* * Squelch classes that this class has been mixed into. */ - FOREACH(mixinSubclassPtr, clsPtr->mixinSubs) { - /* This condition also covers the case where mixinSubclassPtr == - * clsPtr - */ - if (!Deleted(mixinSubclassPtr->thisPtr)) { - Tcl_DeleteCommandFromToken(interp, - mixinSubclassPtr->thisPtr->command); + if (clsPtr->mixinSubs.num > 0) { + while (clsPtr->mixinSubs.num > 0) { + mixinSubclassPtr = clsPtr->mixinSubs.list[clsPtr->mixinSubs.num-1]; + /* This condition also covers the case where mixinSubclassPtr == + * clsPtr + */ + if (!Deleted(mixinSubclassPtr->thisPtr)) { + Tcl_DeleteCommandFromToken(interp, + mixinSubclassPtr->thisPtr->command); + } + TclOORemoveFromMixinSubs(mixinSubclassPtr, clsPtr); } - i -= TclOORemoveFromMixinSubs(mixinSubclassPtr, clsPtr); - TclOODecrRefCount(mixinSubclassPtr->thisPtr); + } + if (clsPtr->mixinSubs.size > 0) { + ckfree(clsPtr->mixinSubs.list); + clsPtr->mixinSubs.size = 0; } /* * Squelch subclasses of this class. */ - FOREACH(subclassPtr, clsPtr->subclasses) { - if (!Deleted(subclassPtr->thisPtr) && !IsRoot(subclassPtr)) { - Tcl_DeleteCommandFromToken(interp, subclassPtr->thisPtr->command); + if (clsPtr->subclasses.num > 0) { + while (clsPtr->subclasses.num > 0) { + subclassPtr = clsPtr->subclasses.list[clsPtr->subclasses.num-1]; + if (!Deleted(subclassPtr->thisPtr) && !IsRoot(subclassPtr)) { + Tcl_DeleteCommandFromToken(interp, subclassPtr->thisPtr->command); + } + TclOORemoveFromSubclasses(subclassPtr, clsPtr); } - i -= TclOORemoveFromSubclasses(subclassPtr, clsPtr); - TclOODecrRefCount(subclassPtr->thisPtr); + } + if (clsPtr->subclasses.size > 0) { + ckfree(clsPtr->subclasses.list); + clsPtr->subclasses.list = NULL; + clsPtr->subclasses.size = 0; } /* * Squelch instances of this class (includes objects we're mixed into). */ - if (!IsRootClass(oPtr)) { - FOREACH(instancePtr, clsPtr->instances) { + if (clsPtr->instances.num > 0) { + while (clsPtr->instances.num > 0) { + instancePtr = clsPtr->instances.list[clsPtr->instances.num-1]; /* This condition also covers the case where instancePtr == oPtr */ if (!Deleted(instancePtr) && !IsRoot(instancePtr)) { Tcl_DeleteCommandFromToken(interp, instancePtr->command); } - i -= TclOORemoveFromInstances(instancePtr, clsPtr); + TclOORemoveFromInstances(instancePtr, clsPtr); } } + if (clsPtr->instances.size > 0) { + ckfree(clsPtr->instances.list); + clsPtr->instances.list = NULL; + clsPtr->instances.size = 0; + } } + +/* + * ---------------------------------------------------------------------- + * + * ReleaseClassContents -- + * + * Tear down the special class data structure, including deleting all + * dependent classes and objects. + * + * ---------------------------------------------------------------------- + */ static void ReleaseClassContents( @@ -948,21 +989,6 @@ ReleaseClassContents( } /* - * Squelch our instances. - */ - - if (clsPtr->instances.num) { - Object *oPtr; - - FOREACH(oPtr, clsPtr->instances) { - TclOODecrRefCount(oPtr); - } - ckfree(clsPtr->instances.list); - clsPtr->instances.list = NULL; - clsPtr->instances.num = 0; - } - - /* * Squelch our metadata. */ @@ -978,11 +1004,21 @@ ReleaseClassContents( clsPtr->metadataPtr = NULL; } - FOREACH(tmpClsPtr, clsPtr->mixins) { - TclOORemoveFromMixinSubs(clsPtr, tmpClsPtr); + if (clsPtr->mixins.num) { + FOREACH(tmpClsPtr, clsPtr->mixins) { + TclOORemoveFromMixinSubs(clsPtr, tmpClsPtr); + } + ckfree(clsPtr->mixins.list); } - FOREACH(tmpClsPtr, clsPtr->superclasses) { - TclOORemoveFromSubclasses(clsPtr, tmpClsPtr); + + if (clsPtr->superclasses.num > 0) { + FOREACH(tmpClsPtr, clsPtr->superclasses) { + TclOORemoveFromSubclasses(clsPtr, tmpClsPtr); + TclOODecrRefCount(tmpClsPtr->thisPtr); + } + ckfree(clsPtr->superclasses.list); + clsPtr->superclasses.num = 0; + clsPtr->superclasses.list = NULL; } FOREACH_HASH_VALUE(mPtr, &clsPtr->classMethods) { @@ -1110,10 +1146,10 @@ ObjectNamespaceDeleted( /* To do: Should this be protected with a * !IsRoot() condition? */ TclOORemoveFromInstances(oPtr, oPtr->selfCls); - FOREACH(mixinPtr, oPtr->mixins) { - i -= TclOORemoveFromInstances(oPtr, mixinPtr); - } - if (i) { + if (oPtr->mixins.num > 0) { + FOREACH(mixinPtr, oPtr->mixins) { + TclOORemoveFromInstances(oPtr, mixinPtr); + } ckfree(oPtr->mixins.list); } @@ -1185,7 +1221,9 @@ ObjectNamespaceDeleted( * Delete the object structure itself. */ + TclNsDecrRefCount((Namespace *)oPtr->namespacePtr); oPtr->namespacePtr = NULL; + TclOODecrRefCount(oPtr->selfCls->thisPtr); oPtr->selfCls = NULL; TclOODecrRefCount(oPtr); return; @@ -1204,13 +1242,7 @@ ObjectNamespaceDeleted( */ int TclOODecrRefCount(Object *oPtr) { if (oPtr->refCount-- <= 1) { - Class *clsPtr = oPtr->classPtr; if (oPtr->classPtr != NULL) { - ckfree(clsPtr->superclasses.list); - ckfree(clsPtr->subclasses.list); - ckfree(clsPtr->instances.list); - ckfree(clsPtr->mixinSubs.list); - ckfree(clsPtr->mixins.list); ckfree(oPtr->classPtr); } ckfree(oPtr); @@ -1250,9 +1282,6 @@ TclOORemoveFromInstances( { int i, res = 0; Object *instPtr; - if (Deleted(clsPtr->thisPtr)) { - return res; - } FOREACH(instPtr, clsPtr->instances) { if (oPtr == instPtr) { @@ -1315,9 +1344,6 @@ TclOORemoveFromSubclasses( { int i, res = 0; Class *subclsPtr; - if (Deleted(superPtr->thisPtr)) { - return res; - } FOREACH(subclsPtr, superPtr->subclasses) { if (subPtr == subclsPtr) { @@ -1382,10 +1408,6 @@ TclOORemoveFromMixinSubs( int i, res = 0; Class *subclsPtr; - if (Deleted(superPtr->thisPtr)) { - return res; - } - FOREACH(subclsPtr, superPtr->mixinSubs) { if (subPtr == subclsPtr) { RemoveItem(Class, superPtr->mixinSubs, i); @@ -1678,6 +1700,7 @@ TclNewObjectInstanceCommon( oPtr = AllocObject(interp, simpleName, nsPtr, nsNameStr); oPtr->selfCls = classPtr; + AddRef(classPtr->thisPtr); TclOOAddToInstances(oPtr, classPtr); /* * Check to see if we're really creating a class. If so, allocate the @@ -1914,6 +1937,11 @@ Tcl_CopyObjectInstance( cls2Ptr->superclasses.num = clsPtr->superclasses.num; FOREACH(superPtr, cls2Ptr->superclasses) { TclOOAddToSubclasses(cls2Ptr, superPtr); + + /* For the new item in cls2Ptr->superclasses that memcpy just + * created + */ + AddRef(superPtr->thisPtr); } /* diff --git a/generic/tclOODefineCmds.c b/generic/tclOODefineCmds.c index d05d899..dfd2acf 100644 --- a/generic/tclOODefineCmds.c +++ b/generic/tclOODefineCmds.c @@ -1125,12 +1125,13 @@ TclOODefineClassObjCmd( */ if (oPtr->selfCls != clsPtr) { - TclOORemoveFromInstances(oPtr, oPtr->selfCls); - /* Reference count already incremented 3 lines up. */ + TclOORemoveFromInstances(oPtr, oPtr->selfCls); + TclOODecrRefCount(oPtr->selfCls->thisPtr); oPtr->selfCls = clsPtr; - + AddRef(oPtr->selfCls->thisPtr); TclOOAddToInstances(oPtr, oPtr->selfCls); + if (oPtr->classPtr != NULL) { BumpGlobalEpoch(interp, oPtr->classPtr); } else { @@ -2151,7 +2152,6 @@ ClassSuperSet( superclasses[0] = oPtr->fPtr->objectCls; } superc = 1; - /* Corresponding TclOODecrRefCount is near the end of this function */ AddRef(superclasses[0]->thisPtr); } else { for (i=0 ; iclassPtr->superclasses.num = superc; FOREACH(superPtr, oPtr->classPtr->superclasses) { TclOOAddToSubclasses(oPtr->classPtr, superPtr); - /* To account for the AddRef() earlier in this function */ - TclOODecrRefCount(superPtr->thisPtr); } BumpGlobalEpoch(interp, oPtr->classPtr); diff --git a/tests/oo.test b/tests/oo.test index 9cf3133..a698bac 100644 --- a/tests/oo.test +++ b/tests/oo.test @@ -57,7 +57,13 @@ test oo-0.4 {basic test of OO's ability to clean up its initial state} -body { foo destroy } } -constraints memory -result 0 -test oo-0.5 {testing literal leak on interp delete} memory { +test oo-0.5.1 {testing object foundation cleanup} memory { + leaktest { + interp create foo + interp delete foo + } +} 0 +test oo-0.5.2 {testing literal leak on interp delete} memory { leaktest { interp create foo foo eval {oo::object new} -- cgit v0.12 From 261fbdac7ec5b605a259cf6c4dc4c2470c7d099d Mon Sep 17 00:00:00 2001 From: dgp Date: Tue, 13 Mar 2018 23:56:22 +0000 Subject: A few minor revisions. --- generic/tclUtil.c | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/generic/tclUtil.c b/generic/tclUtil.c index 96ab96d..58e4cd2 100644 --- a/generic/tclUtil.c +++ b/generic/tclUtil.c @@ -1499,9 +1499,10 @@ Tcl_Backslash( /* *---------------------------------------------------------------------- * - * TclUtfWellFormedEnd -- + * UtfWellFormedEnd -- * Checks the end of utf string is malformed, if yes - wraps bytes - * to the given buffer (as well-formed NTS string). + * to the given buffer (as well-formed NTS string). The buffer + * argument should be initialized by the caller and ready to use. * * Results: * The bytes with well-formed end of the string. @@ -1513,7 +1514,7 @@ Tcl_Backslash( */ static inline const char* -TclUtfWellFormedEnd( +UtfWellFormedEnd( Tcl_DString *buffer, /* Buffer used to hold well-formed string. */ CONST char *bytes, /* Pointer to the beginning of the string. */ int length) /* Length of the string. */ @@ -1522,14 +1523,12 @@ TclUtfWellFormedEnd( CONST char *p = Tcl_UtfPrev(l, bytes); if (Tcl_UtfCharComplete(p, l - p)) { - buffer->string = buffer->staticSpace; return bytes; } /* * Malformed utf-8 end, be sure we've NTS to safe compare of end-character, * avoid segfault by access violation out of range. */ - Tcl_DStringInit(buffer); Tcl_DStringAppend(buffer, bytes, length); return Tcl_DStringValue(buffer); } @@ -1607,8 +1606,10 @@ TclTrimRight( return 0; } - bytes = TclUtfWellFormedEnd(&bytesBuf, bytes, numBytes); - trim = TclUtfWellFormedEnd(&trimBuf, trim, numTrim); + Tcl_DStringInit(&bytesBuf); + Tcl_DStringInit(&trimBuf); + bytes = UtfWellFormedEnd(&bytesBuf, bytes, numBytes); + trim = UtfWellFormedEnd(&trimBuf, trim, numTrim); res = TrimRight(bytes, numBytes, trim, numTrim); if (res > numBytes) { @@ -1694,8 +1695,10 @@ TclTrimLeft( return 0; } - bytes = TclUtfWellFormedEnd(&bytesBuf, bytes, numBytes); - trim = TclUtfWellFormedEnd(&trimBuf, trim, numTrim); + Tcl_DStringInit(&bytesBuf); + Tcl_DStringInit(&trimBuf); + bytes = UtfWellFormedEnd(&bytesBuf, bytes, numBytes); + trim = UtfWellFormedEnd(&trimBuf, trim, numTrim); res = TrimLeft(bytes, numBytes, trim, numTrim); if (res > numBytes) { @@ -1740,8 +1743,10 @@ TclTrim( return 0; } - bytes = TclUtfWellFormedEnd(&bytesBuf, bytes, numBytes); - trim = TclUtfWellFormedEnd(&trimBuf, trim, numTrim); + Tcl_DStringInit(&bytesBuf); + Tcl_DStringInit(&trimBuf); + bytes = UtfWellFormedEnd(&bytesBuf, bytes, numBytes); + trim = UtfWellFormedEnd(&trimBuf, trim, numTrim); trimLeft = TrimLeft(bytes, numBytes, trim, numTrim); if (trimLeft > numBytes) { -- cgit v0.12 From fd2d896c8f9f8e330c209b7a2627cb53f9ff533e Mon Sep 17 00:00:00 2001 From: dgp Date: Wed, 14 Mar 2018 00:45:14 +0000 Subject: TclTrim must write to *trimRight even when making a quick exit. --- generic/tclUtil.c | 1 + 1 file changed, 1 insertion(+) diff --git a/generic/tclUtil.c b/generic/tclUtil.c index 58e4cd2..a8791c5 100644 --- a/generic/tclUtil.c +++ b/generic/tclUtil.c @@ -1740,6 +1740,7 @@ TclTrim( /* Empty strings -> nothing to do */ if ((numBytes == 0) || (numTrim == 0)) { + *trimRight = 0; return 0; } -- cgit v0.12 From 17e2ed6919ccdbd5b8e79be7228635f0b71da3da Mon Sep 17 00:00:00 2001 From: dgp Date: Wed, 14 Mar 2018 01:31:10 +0000 Subject: silence compiler warning --- generic/tclProcess.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/generic/tclProcess.c b/generic/tclProcess.c index 8d98a23..7187ee4 100644 --- a/generic/tclProcess.c +++ b/generic/tclProcess.c @@ -210,7 +210,7 @@ WaitProcessStatus( const char *msg; pid = Tcl_WaitPid(pid, &waitStatus, options); - if ((pid == 0)) { + if (pid == 0) { /* * No change. */ -- cgit v0.12 From b16ec6f23b5b23b658cac89abc3e04ff754a062a Mon Sep 17 00:00:00 2001 From: dgp Date: Wed, 14 Mar 2018 04:26:41 +0000 Subject: Backout recent patch working on Object and Namespace refcounts. It creates new leaks and memory corruption according to valgrind, and the preceding checkin did not. Keep developing on the memleak branch and it can merge again when valgrind and memleak tests both like it. --- generic/tclOO.c | 196 ++++++++++++++++++++-------------------------- generic/tclOODefineCmds.c | 10 ++- tests/oo.test | 8 +- 3 files changed, 91 insertions(+), 123 deletions(-) diff --git a/generic/tclOO.c b/generic/tclOO.c index a129a52..587e46d 100644 --- a/generic/tclOO.c +++ b/generic/tclOO.c @@ -397,39 +397,28 @@ InitFoundation( fPtr->objectCls = AllocClass(interp, AllocObject(interp, "object", (Namespace *)fPtr->ooNs, NULL)); - /* Corresponding TclOODecrRefCount in KillFoudation */ - AddRef(fPtr->objectCls->thisPtr); - - /* This is why it is unnecessary in this routine to replace the - * incremented reference count of fPtr->objectCls that was swallowed by - * fakeObject. */ - fPtr->objectCls->superclasses.num = 0; - ckfree(fPtr->objectCls->superclasses.list); - fPtr->objectCls->superclasses.list = NULL; - - /* special initialization for the primordial objects */ - fPtr->objectCls->thisPtr->flags |= ROOT_OBJECT; - fPtr->objectCls->flags |= ROOT_OBJECT; - fPtr->classCls = AllocClass(interp, AllocObject(interp, "class", (Namespace *)fPtr->ooNs, NULL)); - /* Corresponding TclOODecrRefCount in KillFoudation */ - AddRef(fPtr->classCls->thisPtr); - - /* - * Increment reference counts for each reference because these - * relationships can be dynamically changed. - * - * Corresponding TclOODecrRefCount for all incremented refcounts is in - * KillFoundation. - */ /* Rewire bootstrapped objects. */ fPtr->objectCls->thisPtr->selfCls = fPtr->classCls; - AddRef(fPtr->objectCls->thisPtr->selfCls->thisPtr); - fPtr->classCls->thisPtr->selfCls = fPtr->classCls; + + AddRef(fPtr->objectCls->thisPtr); + AddRef(fPtr->classCls->thisPtr); AddRef(fPtr->classCls->thisPtr->selfCls->thisPtr); + AddRef(fPtr->objectCls->thisPtr->selfCls->thisPtr); + + /* special initialization for the primordial objects */ + fPtr->objectCls->thisPtr->flags |= ROOT_OBJECT; + fPtr->objectCls->flags |= ROOT_OBJECT; + + /* This is why it is unnecessary in this routine to make up for the + * incremented reference count of fPtr->objectCls that was sallwed by + * fakeObject. */ + fPtr->objectCls->superclasses.num = 0; + ckfree(fPtr->objectCls->superclasses.list); + fPtr->objectCls->superclasses.list = NULL; fPtr->classCls->thisPtr->flags |= ROOT_CLASS; fPtr->classCls->flags |= ROOT_CLASS; @@ -563,20 +552,20 @@ KillFoundation( { Foundation *fPtr = GetFoundation(interp); + /* + * Crude mechanism to avoid leaking the Object struct of the + * foundation components oo::object and oo::class + * + * Should probably be replaced with something more elegantly designed. + */ + while (TclOODecrRefCount(fPtr->objectCls->thisPtr) == 0) {}; + while (TclOODecrRefCount(fPtr->classCls->thisPtr) == 0) {}; + TclDecrRefCount(fPtr->unknownMethodNameObj); TclDecrRefCount(fPtr->constructorName); TclDecrRefCount(fPtr->destructorName); TclDecrRefCount(fPtr->clonedName); TclDecrRefCount(fPtr->defineName); - if (fPtr->objectCls->thisPtr->selfCls != NULL) { - TclOODecrRefCount(fPtr->objectCls->thisPtr->selfCls->thisPtr); - } - if (fPtr->classCls->thisPtr->selfCls != NULL) { - TclOODecrRefCount(fPtr->classCls->thisPtr->selfCls->thisPtr); - } - TclOODecrRefCount(fPtr->objectCls->thisPtr); - TclOODecrRefCount(fPtr->classCls->thisPtr); - ckfree(fPtr); } @@ -660,8 +649,6 @@ AllocObject( Tcl_ResetResult(interp); } - ((Namespace *)oPtr->namespacePtr)->refCount++; - /* * Make the namespace know about the helper commands. This grants access * to the [self] and [next] commands. @@ -833,9 +820,10 @@ ObjectRenamedTrace( /* * ---------------------------------------------------------------------- * - * DeleteDescendants -- + * ReleaseClassContents -- * - * Delete all descendants of a particular class. + * Tear down the special class data structure, including deleting all + * dependent classes and objects. * * ---------------------------------------------------------------------- */ @@ -847,79 +835,50 @@ DeleteDescendants( { Class *clsPtr = oPtr->classPtr, *subclassPtr, *mixinSubclassPtr; Object *instancePtr; + int i; /* * Squelch classes that this class has been mixed into. */ - if (clsPtr->mixinSubs.num > 0) { - while (clsPtr->mixinSubs.num > 0) { - mixinSubclassPtr = clsPtr->mixinSubs.list[clsPtr->mixinSubs.num-1]; - /* This condition also covers the case where mixinSubclassPtr == - * clsPtr - */ - if (!Deleted(mixinSubclassPtr->thisPtr)) { - Tcl_DeleteCommandFromToken(interp, - mixinSubclassPtr->thisPtr->command); - } - TclOORemoveFromMixinSubs(mixinSubclassPtr, clsPtr); + FOREACH(mixinSubclassPtr, clsPtr->mixinSubs) { + /* This condition also covers the case where mixinSubclassPtr == + * clsPtr + */ + if (!Deleted(mixinSubclassPtr->thisPtr)) { + Tcl_DeleteCommandFromToken(interp, + mixinSubclassPtr->thisPtr->command); } - } - if (clsPtr->mixinSubs.size > 0) { - ckfree(clsPtr->mixinSubs.list); - clsPtr->mixinSubs.size = 0; + i -= TclOORemoveFromMixinSubs(mixinSubclassPtr, clsPtr); + TclOODecrRefCount(mixinSubclassPtr->thisPtr); } /* * Squelch subclasses of this class. */ - if (clsPtr->subclasses.num > 0) { - while (clsPtr->subclasses.num > 0) { - subclassPtr = clsPtr->subclasses.list[clsPtr->subclasses.num-1]; - if (!Deleted(subclassPtr->thisPtr) && !IsRoot(subclassPtr)) { - Tcl_DeleteCommandFromToken(interp, subclassPtr->thisPtr->command); - } - TclOORemoveFromSubclasses(subclassPtr, clsPtr); + FOREACH(subclassPtr, clsPtr->subclasses) { + if (!Deleted(subclassPtr->thisPtr) && !IsRoot(subclassPtr)) { + Tcl_DeleteCommandFromToken(interp, subclassPtr->thisPtr->command); } - } - if (clsPtr->subclasses.size > 0) { - ckfree(clsPtr->subclasses.list); - clsPtr->subclasses.list = NULL; - clsPtr->subclasses.size = 0; + i -= TclOORemoveFromSubclasses(subclassPtr, clsPtr); + TclOODecrRefCount(subclassPtr->thisPtr); } /* * Squelch instances of this class (includes objects we're mixed into). */ - if (clsPtr->instances.num > 0) { - while (clsPtr->instances.num > 0) { - instancePtr = clsPtr->instances.list[clsPtr->instances.num-1]; + if (!IsRootClass(oPtr)) { + FOREACH(instancePtr, clsPtr->instances) { /* This condition also covers the case where instancePtr == oPtr */ if (!Deleted(instancePtr) && !IsRoot(instancePtr)) { Tcl_DeleteCommandFromToken(interp, instancePtr->command); } - TclOORemoveFromInstances(instancePtr, clsPtr); + i -= TclOORemoveFromInstances(instancePtr, clsPtr); } } - if (clsPtr->instances.size > 0) { - ckfree(clsPtr->instances.list); - clsPtr->instances.list = NULL; - clsPtr->instances.size = 0; - } } - -/* - * ---------------------------------------------------------------------- - * - * ReleaseClassContents -- - * - * Tear down the special class data structure, including deleting all - * dependent classes and objects. - * - * ---------------------------------------------------------------------- - */ static void ReleaseClassContents( @@ -989,6 +948,21 @@ ReleaseClassContents( } /* + * Squelch our instances. + */ + + if (clsPtr->instances.num) { + Object *oPtr; + + FOREACH(oPtr, clsPtr->instances) { + TclOODecrRefCount(oPtr); + } + ckfree(clsPtr->instances.list); + clsPtr->instances.list = NULL; + clsPtr->instances.num = 0; + } + + /* * Squelch our metadata. */ @@ -1004,21 +978,11 @@ ReleaseClassContents( clsPtr->metadataPtr = NULL; } - if (clsPtr->mixins.num) { - FOREACH(tmpClsPtr, clsPtr->mixins) { - TclOORemoveFromMixinSubs(clsPtr, tmpClsPtr); - } - ckfree(clsPtr->mixins.list); + FOREACH(tmpClsPtr, clsPtr->mixins) { + TclOORemoveFromMixinSubs(clsPtr, tmpClsPtr); } - - if (clsPtr->superclasses.num > 0) { - FOREACH(tmpClsPtr, clsPtr->superclasses) { - TclOORemoveFromSubclasses(clsPtr, tmpClsPtr); - TclOODecrRefCount(tmpClsPtr->thisPtr); - } - ckfree(clsPtr->superclasses.list); - clsPtr->superclasses.num = 0; - clsPtr->superclasses.list = NULL; + FOREACH(tmpClsPtr, clsPtr->superclasses) { + TclOORemoveFromSubclasses(clsPtr, tmpClsPtr); } FOREACH_HASH_VALUE(mPtr, &clsPtr->classMethods) { @@ -1146,10 +1110,10 @@ ObjectNamespaceDeleted( /* To do: Should this be protected with a * !IsRoot() condition? */ TclOORemoveFromInstances(oPtr, oPtr->selfCls); - if (oPtr->mixins.num > 0) { - FOREACH(mixinPtr, oPtr->mixins) { - TclOORemoveFromInstances(oPtr, mixinPtr); - } + FOREACH(mixinPtr, oPtr->mixins) { + i -= TclOORemoveFromInstances(oPtr, mixinPtr); + } + if (i) { ckfree(oPtr->mixins.list); } @@ -1221,9 +1185,7 @@ ObjectNamespaceDeleted( * Delete the object structure itself. */ - TclNsDecrRefCount((Namespace *)oPtr->namespacePtr); oPtr->namespacePtr = NULL; - TclOODecrRefCount(oPtr->selfCls->thisPtr); oPtr->selfCls = NULL; TclOODecrRefCount(oPtr); return; @@ -1242,7 +1204,13 @@ ObjectNamespaceDeleted( */ int TclOODecrRefCount(Object *oPtr) { if (oPtr->refCount-- <= 1) { + Class *clsPtr = oPtr->classPtr; if (oPtr->classPtr != NULL) { + ckfree(clsPtr->superclasses.list); + ckfree(clsPtr->subclasses.list); + ckfree(clsPtr->instances.list); + ckfree(clsPtr->mixinSubs.list); + ckfree(clsPtr->mixins.list); ckfree(oPtr->classPtr); } ckfree(oPtr); @@ -1282,6 +1250,9 @@ TclOORemoveFromInstances( { int i, res = 0; Object *instPtr; + if (Deleted(clsPtr->thisPtr)) { + return res; + } FOREACH(instPtr, clsPtr->instances) { if (oPtr == instPtr) { @@ -1344,6 +1315,9 @@ TclOORemoveFromSubclasses( { int i, res = 0; Class *subclsPtr; + if (Deleted(superPtr->thisPtr)) { + return res; + } FOREACH(subclsPtr, superPtr->subclasses) { if (subPtr == subclsPtr) { @@ -1408,6 +1382,10 @@ TclOORemoveFromMixinSubs( int i, res = 0; Class *subclsPtr; + if (Deleted(superPtr->thisPtr)) { + return res; + } + FOREACH(subclsPtr, superPtr->mixinSubs) { if (subPtr == subclsPtr) { RemoveItem(Class, superPtr->mixinSubs, i); @@ -1700,7 +1678,6 @@ TclNewObjectInstanceCommon( oPtr = AllocObject(interp, simpleName, nsPtr, nsNameStr); oPtr->selfCls = classPtr; - AddRef(classPtr->thisPtr); TclOOAddToInstances(oPtr, classPtr); /* * Check to see if we're really creating a class. If so, allocate the @@ -1937,11 +1914,6 @@ Tcl_CopyObjectInstance( cls2Ptr->superclasses.num = clsPtr->superclasses.num; FOREACH(superPtr, cls2Ptr->superclasses) { TclOOAddToSubclasses(cls2Ptr, superPtr); - - /* For the new item in cls2Ptr->superclasses that memcpy just - * created - */ - AddRef(superPtr->thisPtr); } /* diff --git a/generic/tclOODefineCmds.c b/generic/tclOODefineCmds.c index dfd2acf..d05d899 100644 --- a/generic/tclOODefineCmds.c +++ b/generic/tclOODefineCmds.c @@ -1125,13 +1125,12 @@ TclOODefineClassObjCmd( */ if (oPtr->selfCls != clsPtr) { - TclOORemoveFromInstances(oPtr, oPtr->selfCls); - TclOODecrRefCount(oPtr->selfCls->thisPtr); + + /* Reference count already incremented 3 lines up. */ oPtr->selfCls = clsPtr; - AddRef(oPtr->selfCls->thisPtr); - TclOOAddToInstances(oPtr, oPtr->selfCls); + TclOOAddToInstances(oPtr, oPtr->selfCls); if (oPtr->classPtr != NULL) { BumpGlobalEpoch(interp, oPtr->classPtr); } else { @@ -2152,6 +2151,7 @@ ClassSuperSet( superclasses[0] = oPtr->fPtr->objectCls; } superc = 1; + /* Corresponding TclOODecrRefCount is near the end of this function */ AddRef(superclasses[0]->thisPtr); } else { for (i=0 ; iclassPtr->superclasses.num = superc; FOREACH(superPtr, oPtr->classPtr->superclasses) { TclOOAddToSubclasses(oPtr->classPtr, superPtr); + /* To account for the AddRef() earlier in this function */ + TclOODecrRefCount(superPtr->thisPtr); } BumpGlobalEpoch(interp, oPtr->classPtr); diff --git a/tests/oo.test b/tests/oo.test index a698bac..9cf3133 100644 --- a/tests/oo.test +++ b/tests/oo.test @@ -57,13 +57,7 @@ test oo-0.4 {basic test of OO's ability to clean up its initial state} -body { foo destroy } } -constraints memory -result 0 -test oo-0.5.1 {testing object foundation cleanup} memory { - leaktest { - interp create foo - interp delete foo - } -} 0 -test oo-0.5.2 {testing literal leak on interp delete} memory { +test oo-0.5 {testing literal leak on interp delete} memory { leaktest { interp create foo foo eval {oo::object new} -- cgit v0.12 From 864d754a8a83ea3c7e345ffa6bc41acee6422ad3 Mon Sep 17 00:00:00 2001 From: dgp Date: Wed, 14 Mar 2018 04:45:29 +0000 Subject: backout the latest merge --- generic/tclCmdMZ.c | 3 +- generic/tclInt.h | 2 - generic/tclOO.c | 211 ++++++++++++++++++-------------------- generic/tclOODefineCmds.c | 21 +++- generic/tclUtil.c | 255 +++++++++++++--------------------------------- tests/oo.test | 8 +- 6 files changed, 191 insertions(+), 309 deletions(-) diff --git a/generic/tclCmdMZ.c b/generic/tclCmdMZ.c index 3c5c5e4..ba96d7c 100644 --- a/generic/tclCmdMZ.c +++ b/generic/tclCmdMZ.c @@ -3228,7 +3228,8 @@ StringTrimCmd( } string1 = TclGetStringFromObj(objv[1], &length1); - triml = TclTrim(string1, length1, string2, length2, &trimr); + triml = TclTrimLeft(string1, length1, string2, length2); + trimr = TclTrimRight(string1 + triml, length1 - triml, string2, length2); Tcl_SetObjResult(interp, Tcl_NewStringObj(string1 + triml, length1 - triml - trimr)); diff --git a/generic/tclInt.h b/generic/tclInt.h index 57f648c..e7794c7 100644 --- a/generic/tclInt.h +++ b/generic/tclInt.h @@ -3198,8 +3198,6 @@ MODULE_SCOPE void TclSubstParse(Tcl_Interp *interp, const char *bytes, MODULE_SCOPE int TclSubstTokens(Tcl_Interp *interp, Tcl_Token *tokenPtr, int count, int *tokensLeftPtr, int line, int *clNextOuter, const char *outerScript); -MODULE_SCOPE int TclTrim(const char *bytes, int numBytes, - const char *trim, int numTrim, int *trimRight); MODULE_SCOPE int TclTrimLeft(const char *bytes, int numBytes, const char *trim, int numTrim); MODULE_SCOPE int TclTrimRight(const char *bytes, int numBytes, diff --git a/generic/tclOO.c b/generic/tclOO.c index c80f039..dcf48ef 100644 --- a/generic/tclOO.c +++ b/generic/tclOO.c @@ -523,47 +523,47 @@ InitClassSystemRoots( fakeCls.thisPtr = &fakeObject; - fPtr->objectCls = AllocClass(interp, AllocObject(interp, "object", (Namespace *)fPtr->ooNs, NULL)); - /* Corresponding TclOODecrRefCount in KillFoudation */ - AddRef(fPtr->objectCls->thisPtr); - - /* This is why it is unnecessary in this routine to replace the - * incremented reference count of fPtr->objectCls that was swallowed by - * fakeObject. */ - fPtr->objectCls->superclasses.num = 0; - ckfree(fPtr->objectCls->superclasses.list); - fPtr->objectCls->superclasses.list = NULL; - - /* special initialization for the primordial objects */ - fPtr->objectCls->thisPtr->flags |= ROOT_OBJECT; - fPtr->objectCls->flags |= ROOT_OBJECT; - fPtr->classCls = AllocClass(interp, AllocObject(interp, "class", (Namespace *)fPtr->ooNs, NULL)); - /* Corresponding TclOODecrRefCount in KillFoudation */ - AddRef(fPtr->classCls->thisPtr); /* - * Increment reference counts for each reference because these - * relationships can be dynamically changed. - * - * Corresponding TclOODecrRefCount for all incremented refcounts is in - * KillFoundation. + * Rewire bootstrapped objects. */ - /* Rewire bootstrapped objects. */ fPtr->objectCls->thisPtr->selfCls = fPtr->classCls; - AddRef(fPtr->objectCls->thisPtr->selfCls->thisPtr); - fPtr->classCls->thisPtr->selfCls = fPtr->classCls; + + AddRef(fPtr->objectCls->thisPtr); + AddRef(fPtr->classCls->thisPtr); AddRef(fPtr->classCls->thisPtr->selfCls->thisPtr); + AddRef(fPtr->objectCls->thisPtr->selfCls->thisPtr); + + /* + * Special initialization for the primordial objects. + */ + + fPtr->objectCls->thisPtr->flags |= ROOT_OBJECT; + fPtr->objectCls->flags |= ROOT_OBJECT; + + /* + * This is why it is unnecessary in this routine to make up for the + * incremented reference count of fPtr->objectCls that was sallwed by + * fakeObject. + */ + + fPtr->objectCls->superclasses.num = 0; + ckfree(fPtr->objectCls->superclasses.list); + fPtr->objectCls->superclasses.list = NULL; fPtr->classCls->thisPtr->flags |= ROOT_CLASS; fPtr->classCls->flags |= ROOT_CLASS; - /* Standard initialization for new Objects */ + /* + * Standard initialization for new Objects. + */ + TclOOAddToInstances(fPtr->objectCls->thisPtr, fPtr->classCls); TclOOAddToInstances(fPtr->classCls->thisPtr, fPtr->classCls); TclOOAddToSubclasses(fPtr->classCls, fPtr->objectCls); @@ -632,20 +632,20 @@ KillFoundation( { Foundation *fPtr = GetFoundation(interp); + /* + * Crude mechanism to avoid leaking the Object struct of the + * foundation components oo::object and oo::class + * + * Should probably be replaced with something more elegantly designed. + */ + while (TclOODecrRefCount(fPtr->objectCls->thisPtr) == 0) {}; + while (TclOODecrRefCount(fPtr->classCls->thisPtr) == 0) {}; + TclDecrRefCount(fPtr->unknownMethodNameObj); TclDecrRefCount(fPtr->constructorName); TclDecrRefCount(fPtr->destructorName); TclDecrRefCount(fPtr->clonedName); TclDecrRefCount(fPtr->defineName); - if (fPtr->objectCls->thisPtr->selfCls != NULL) { - TclOODecrRefCount(fPtr->objectCls->thisPtr->selfCls->thisPtr); - } - if (fPtr->classCls->thisPtr->selfCls != NULL) { - TclOODecrRefCount(fPtr->classCls->thisPtr->selfCls->thisPtr); - } - TclOODecrRefCount(fPtr->objectCls->thisPtr); - TclOODecrRefCount(fPtr->classCls->thisPtr); - ckfree(fPtr); } @@ -729,8 +729,6 @@ AllocObject( Tcl_ResetResult(interp); } - ((Namespace *)oPtr->namespacePtr)->refCount++; - /* * Make the namespace know about the helper commands. This grants access * to the [self] and [next] commands. @@ -903,9 +901,10 @@ ObjectRenamedTrace( /* * ---------------------------------------------------------------------- * - * DeleteDescendants -- + * DeleteDescendants, ReleaseClassContents -- * - * Delete all descendants of a particular class. + * Tear down the special class data structure, including deleting all + * dependent classes and objects. * * ---------------------------------------------------------------------- */ @@ -917,55 +916,44 @@ DeleteDescendants( { Class *clsPtr = oPtr->classPtr, *subclassPtr, *mixinSubclassPtr; Object *instancePtr; + int i; /* * Squelch classes that this class has been mixed into. */ - if (clsPtr->mixinSubs.num > 0) { - while (clsPtr->mixinSubs.num > 0) { - mixinSubclassPtr = clsPtr->mixinSubs.list[clsPtr->mixinSubs.num-1]; - /* This condition also covers the case where mixinSubclassPtr == - * clsPtr - */ - if (!Deleted(mixinSubclassPtr->thisPtr)) { - Tcl_DeleteCommandFromToken(interp, - mixinSubclassPtr->thisPtr->command); - } - TclOORemoveFromMixinSubs(mixinSubclassPtr, clsPtr); + FOREACH(mixinSubclassPtr, clsPtr->mixinSubs) { + /* + * This condition also covers the case where mixinSubclassPtr == + * clsPtr + */ + + if (!Deleted(mixinSubclassPtr->thisPtr)) { + Tcl_DeleteCommandFromToken(interp, + mixinSubclassPtr->thisPtr->command); } - } - if (clsPtr->mixinSubs.size > 0) { - ckfree(clsPtr->mixinSubs.list); - clsPtr->mixinSubs.size = 0; + i -= TclOORemoveFromMixinSubs(mixinSubclassPtr, clsPtr); + TclOODecrRefCount(mixinSubclassPtr->thisPtr); } /* * Squelch subclasses of this class. */ - if (clsPtr->subclasses.num > 0) { - while (clsPtr->subclasses.num > 0) { - subclassPtr = clsPtr->subclasses.list[clsPtr->subclasses.num-1]; - if (!Deleted(subclassPtr->thisPtr) && !IsRoot(subclassPtr)) { - Tcl_DeleteCommandFromToken(interp, subclassPtr->thisPtr->command); - } - TclOORemoveFromSubclasses(subclassPtr, clsPtr); + FOREACH(subclassPtr, clsPtr->subclasses) { + if (!Deleted(subclassPtr->thisPtr) && !IsRoot(subclassPtr)) { + Tcl_DeleteCommandFromToken(interp, subclassPtr->thisPtr->command); } - } - if (clsPtr->subclasses.size > 0) { - ckfree(clsPtr->subclasses.list); - clsPtr->subclasses.list = NULL; - clsPtr->subclasses.size = 0; + i -= TclOORemoveFromSubclasses(subclassPtr, clsPtr); + TclOODecrRefCount(subclassPtr->thisPtr); } /* * Squelch instances of this class (includes objects we're mixed into). */ - if (clsPtr->instances.num > 0) { - while (clsPtr->instances.num > 0) { - instancePtr = clsPtr->instances.list[clsPtr->instances.num-1]; + if (!IsRootClass(oPtr)) { + FOREACH(instancePtr, clsPtr->instances) { /* * This condition also covers the case where instancePtr == oPtr */ @@ -973,26 +961,10 @@ DeleteDescendants( if (!Deleted(instancePtr) && !IsRoot(instancePtr)) { Tcl_DeleteCommandFromToken(interp, instancePtr->command); } - TclOORemoveFromInstances(instancePtr, clsPtr); + i -= TclOORemoveFromInstances(instancePtr, clsPtr); } } - if (clsPtr->instances.size > 0) { - ckfree(clsPtr->instances.list); - clsPtr->instances.list = NULL; - clsPtr->instances.size = 0; - } } - -/* - * ---------------------------------------------------------------------- - * - * ReleaseClassContents -- - * - * Tear down the special class data structure, including deleting all - * dependent classes and objects. - * - * ---------------------------------------------------------------------- - */ static void ReleaseClassContents( @@ -1062,6 +1034,21 @@ ReleaseClassContents( } /* + * Squelch our instances. + */ + + if (clsPtr->instances.num) { + Object *oPtr; + + FOREACH(oPtr, clsPtr->instances) { + TclOODecrRefCount(oPtr); + } + ckfree(clsPtr->instances.list); + clsPtr->instances.list = NULL; + clsPtr->instances.num = 0; + } + + /* * Squelch our metadata. */ @@ -1077,21 +1064,11 @@ ReleaseClassContents( clsPtr->metadataPtr = NULL; } - if (clsPtr->mixins.num) { - FOREACH(tmpClsPtr, clsPtr->mixins) { - TclOORemoveFromMixinSubs(clsPtr, tmpClsPtr); - } - ckfree(clsPtr->mixins.list); + FOREACH(tmpClsPtr, clsPtr->mixins) { + TclOORemoveFromMixinSubs(clsPtr, tmpClsPtr); } - - if (clsPtr->superclasses.num > 0) { - FOREACH(tmpClsPtr, clsPtr->superclasses) { - TclOORemoveFromSubclasses(clsPtr, tmpClsPtr); - TclOODecrRefCount(tmpClsPtr->thisPtr); - } - ckfree(clsPtr->superclasses.list); - clsPtr->superclasses.num = 0; - clsPtr->superclasses.list = NULL; + FOREACH(tmpClsPtr, clsPtr->superclasses) { + TclOORemoveFromSubclasses(clsPtr, tmpClsPtr); } FOREACH_HASH_VALUE(mPtr, &clsPtr->classMethods) { @@ -1227,10 +1204,10 @@ ObjectNamespaceDeleted( TclOORemoveFromInstances(oPtr, oPtr->selfCls); - if (oPtr->mixins.num > 0) { - FOREACH(mixinPtr, oPtr->mixins) { - TclOORemoveFromInstances(oPtr, mixinPtr); - } + FOREACH(mixinPtr, oPtr->mixins) { + i -= TclOORemoveFromInstances(oPtr, mixinPtr); + } + if (i) { ckfree(oPtr->mixins.list); } @@ -1299,9 +1276,7 @@ ObjectNamespaceDeleted( * Delete the object structure itself. */ - TclNsDecrRefCount((Namespace *)oPtr->namespacePtr); oPtr->namespacePtr = NULL; - TclOODecrRefCount(oPtr->selfCls->thisPtr); oPtr->selfCls = NULL; TclOODecrRefCount(oPtr); return; @@ -1324,8 +1299,14 @@ TclOODecrRefCount( Object *oPtr) { if (oPtr->refCount-- <= 1) { + Class *clsPtr = oPtr->classPtr; if (oPtr->classPtr != NULL) { + ckfree(clsPtr->superclasses.list); + ckfree(clsPtr->subclasses.list); + ckfree(clsPtr->instances.list); + ckfree(clsPtr->mixinSubs.list); + ckfree(clsPtr->mixins.list); ckfree(oPtr->classPtr); } ckfree(oPtr); @@ -1354,6 +1335,10 @@ TclOORemoveFromInstances( int i, res = 0; Object *instPtr; + if (Deleted(clsPtr->thisPtr)) { + return res; + } + FOREACH(instPtr, clsPtr->instances) { if (oPtr == instPtr) { RemoveItem(Object, clsPtr->instances, i); @@ -1416,6 +1401,10 @@ TclOORemoveFromSubclasses( int i, res = 0; Class *subclsPtr; + if (Deleted(superPtr->thisPtr)) { + return res; + } + FOREACH(subclsPtr, superPtr->subclasses) { if (subPtr == subclsPtr) { RemoveItem(Class, superPtr->subclasses, i); @@ -1480,6 +1469,10 @@ TclOORemoveFromMixinSubs( int i, res = 0; Class *subclsPtr; + if (Deleted(superPtr->thisPtr)) { + return res; + } + FOREACH(subclsPtr, superPtr->mixinSubs) { if (subPtr == subclsPtr) { RemoveItem(Class, superPtr->mixinSubs, i); @@ -1787,7 +1780,6 @@ TclNewObjectInstanceCommon( oPtr = AllocObject(interp, simpleName, nsPtr, nsNameStr); oPtr->selfCls = classPtr; - AddRef(classPtr->thisPtr); TclOOAddToInstances(oPtr, classPtr); /* @@ -2033,11 +2025,6 @@ Tcl_CopyObjectInstance( cls2Ptr->superclasses.num = clsPtr->superclasses.num; FOREACH(superPtr, cls2Ptr->superclasses) { TclOOAddToSubclasses(cls2Ptr, superPtr); - - /* For the new item in cls2Ptr->superclasses that memcpy just - * created - */ - AddRef(superPtr->thisPtr); } /* diff --git a/generic/tclOODefineCmds.c b/generic/tclOODefineCmds.c index 6c1d58a..7c2a641 100644 --- a/generic/tclOODefineCmds.c +++ b/generic/tclOODefineCmds.c @@ -1185,13 +1185,15 @@ TclOODefineClassObjCmd( */ if (oPtr->selfCls != clsPtr) { - TclOORemoveFromInstances(oPtr, oPtr->selfCls); - TclOODecrRefCount(oPtr->selfCls->thisPtr); + + /* + * Reference count already incremented a few lines up. + */ + oPtr->selfCls = clsPtr; - AddRef(oPtr->selfCls->thisPtr); - TclOOAddToInstances(oPtr, oPtr->selfCls); + TclOOAddToInstances(oPtr, oPtr->selfCls); if (oPtr->classPtr != NULL) { BumpGlobalEpoch(interp, oPtr->classPtr); } else { @@ -2232,6 +2234,11 @@ ClassSuperSet( superclasses[0] = oPtr->fPtr->objectCls; } superc = 1; + + /* + * Corresponding TclOODecrRefCount is near the end of this function. + */ + AddRef(superclasses[0]->thisPtr); } else { for (i=0 ; iclassPtr->superclasses.num = superc; FOREACH(superPtr, oPtr->classPtr->superclasses) { TclOOAddToSubclasses(oPtr->classPtr, superPtr); + + /* + * To account for the AddRef() earlier in this function. + */ + + TclOODecrRefCount(superPtr->thisPtr); } BumpGlobalEpoch(interp, oPtr->classPtr); diff --git a/generic/tclUtil.c b/generic/tclUtil.c index 3fbc325..d1b81a9 100644 --- a/generic/tclUtil.c +++ b/generic/tclUtil.c @@ -1647,46 +1647,11 @@ Tcl_Backslash( /* *---------------------------------------------------------------------- * - * UtfWellFormedEnd -- - * Checks the end of utf string is malformed, if yes - wraps bytes - * to the given buffer (as well-formed NTS string). The buffer - * argument should be initialized by the caller and ready to use. - * - * Results: - * The bytes with well-formed end of the string. - * - * Side effects: - * Buffer (DString) may be allocated, so must be released. - * - *---------------------------------------------------------------------- - */ - -static inline const char* -UtfWellFormedEnd( - Tcl_DString *buffer, /* Buffer used to hold well-formed string. */ - const char *bytes, /* Pointer to the beginning of the string. */ - int length) /* Length of the string. */ -{ - const char *l = bytes + length; - const char *p = Tcl_UtfPrev(l, bytes); - - if (Tcl_UtfCharComplete(p, l - p)) { - return bytes; - } - /* - * Malformed utf-8 end, be sure we've NTS to safe compare of end-character, - * avoid segfault by access violation out of range. - */ - Tcl_DStringAppend(buffer, bytes, length); - return Tcl_DStringValue(buffer); -} -/* - *---------------------------------------------------------------------- - * * TclTrimRight -- - * Takes two counted strings in the Tcl encoding. Conceptually - * finds the sub string (offset) to trim from the right side of the - * first string all characters found in the second string. + * + * Takes two counted strings in the Tcl encoding which must both be null + * terminated. Conceptually trims from the right side of the first string + * all characters found in the second string. * * Results: * The number of bytes to be removed from the end of the string. @@ -1697,8 +1662,8 @@ UtfWellFormedEnd( *---------------------------------------------------------------------- */ -static inline int -TrimRight( +int +TclTrimRight( const char *bytes, /* String to be trimmed... */ int numBytes, /* ...and its length in bytes */ const char *trim, /* String of trim characters... */ @@ -1708,6 +1673,18 @@ TrimRight( int pInc; Tcl_UniChar ch1 = 0, ch2 = 0; + if ((bytes[numBytes] != '\0') || (trim[numTrim] != '\0')) { + Tcl_Panic("TclTrimRight works only on null-terminated strings"); + } + + /* + * Empty strings -> nothing to do. + */ + + if ((numBytes == 0) || (numTrim == 0)) { + return 0; + } + /* * Outer loop: iterate over string to be trimmed. */ @@ -1746,46 +1723,15 @@ TrimRight( return numBytes - (p - bytes); } - -int -TclTrimRight( - const char *bytes, /* String to be trimmed... */ - int numBytes, /* ...and its length in bytes */ - const char *trim, /* String of trim characters... */ - int numTrim) /* ...and its length in bytes */ -{ - int res; - Tcl_DString bytesBuf, trimBuf; - - /* Empty strings -> nothing to do */ - if ((numBytes == 0) || (numTrim == 0)) { - return 0; - } - - Tcl_DStringInit(&bytesBuf); - Tcl_DStringInit(&trimBuf); - bytes = UtfWellFormedEnd(&bytesBuf, bytes, numBytes); - trim = UtfWellFormedEnd(&trimBuf, trim, numTrim); - - res = TrimRight(bytes, numBytes, trim, numTrim); - if (res > numBytes) { - res = numBytes; - } - - Tcl_DStringFree(&bytesBuf); - Tcl_DStringFree(&trimBuf); - - return res; -} /* *---------------------------------------------------------------------- * * TclTrimLeft -- * - * Takes two counted strings in the Tcl encoding. Conceptually - * finds the sub string (offset) to trim from the left side of the - * first string all characters found in the second string. + * Takes two counted strings in the Tcl encoding which must both be null + * terminated. Conceptually trims from the left side of the first string + * all characters found in the second string. * * Results: * The number of bytes to be removed from the start of the string. @@ -1796,8 +1742,8 @@ TclTrimRight( *---------------------------------------------------------------------- */ -static inline int -TrimLeft( +int +TclTrimLeft( const char *bytes, /* String to be trimmed... */ int numBytes, /* ...and its length in bytes */ const char *trim, /* String of trim characters... */ @@ -1806,6 +1752,18 @@ TrimLeft( const char *p = bytes; Tcl_UniChar ch1 = 0, ch2 = 0; + if ((bytes[numBytes] != '\0') || (trim[numTrim] != '\0')) { + Tcl_Panic("TclTrimLeft works only on null-terminated strings"); + } + + /* + * Empty strings -> nothing to do. + */ + + if ((numBytes == 0) || (numTrim == 0)) { + return 0; + } + /* * Outer loop: iterate over string to be trimmed. */ @@ -1840,99 +1798,10 @@ TrimLeft( p += pInc; numBytes -= pInc; - } while (numBytes > 0); + } while (numBytes); return p - bytes; } - -int -TclTrimLeft( - const char *bytes, /* String to be trimmed... */ - int numBytes, /* ...and its length in bytes */ - const char *trim, /* String of trim characters... */ - int numTrim) /* ...and its length in bytes */ -{ - int res; - Tcl_DString bytesBuf, trimBuf; - - /* Empty strings -> nothing to do */ - if ((numBytes == 0) || (numTrim == 0)) { - return 0; - } - - Tcl_DStringInit(&bytesBuf); - Tcl_DStringInit(&trimBuf); - bytes = UtfWellFormedEnd(&bytesBuf, bytes, numBytes); - trim = UtfWellFormedEnd(&trimBuf, trim, numTrim); - - res = TrimLeft(bytes, numBytes, trim, numTrim); - if (res > numBytes) { - res = numBytes; - } - - Tcl_DStringFree(&bytesBuf); - Tcl_DStringFree(&trimBuf); - - return res; -} - -/* - *---------------------------------------------------------------------- - * - * TclTrim -- - * Finds the sub string (offset) to trim from both sides of the - * first string all characters found in the second string. - * - * Results: - * The number of bytes to be removed from the start of the string - * - * Side effects: - * None. - * - *---------------------------------------------------------------------- - */ - -int -TclTrim( - const char *bytes, /* String to be trimmed... */ - int numBytes, /* ...and its length in bytes */ - const char *trim, /* String of trim characters... */ - int numTrim, /* ...and its length in bytes */ - int *trimRight) /* Offset from the end of the string. */ -{ - int trimLeft; - Tcl_DString bytesBuf, trimBuf; - - /* Empty strings -> nothing to do */ - if ((numBytes == 0) || (numTrim == 0)) { - *trimRight = 0; - return 0; - } - - Tcl_DStringInit(&bytesBuf); - Tcl_DStringInit(&trimBuf); - bytes = UtfWellFormedEnd(&bytesBuf, bytes, numBytes); - trim = UtfWellFormedEnd(&trimBuf, trim, numTrim); - - trimLeft = TrimLeft(bytes, numBytes, trim, numTrim); - if (trimLeft > numBytes) { - trimLeft = numBytes; - } - numBytes -= trimLeft; - *trimRight = 0; - if (numBytes) { - bytes += trimLeft; - *trimRight = TrimRight(bytes, numBytes, trim, numTrim); - if (*trimRight > numBytes) { - *trimRight = numBytes; - } - } - - Tcl_DStringFree(&bytesBuf); - Tcl_DStringFree(&trimBuf); - - return trimLeft; -} /* *---------------------------------------------------------------------- @@ -2000,20 +1869,30 @@ Tcl_Concat( result = ckalloc((unsigned) (bytesNeeded + argc)); for (p = result, i = 0; i < argc; i++) { - int triml, trimr, elemLength; + int trim, elemLength; const char *element; element = argv[i]; elemLength = strlen(argv[i]); - /* Trim away the leading/trailing whitespace. */ - triml = TclTrim(element, elemLength, CONCAT_TRIM_SET, - CONCAT_WS_SIZE, &trimr); - element += triml; - elemLength -= triml + trimr; + /* + * Trim away the leading whitespace. + */ + + trim = TclTrimLeft(element, elemLength, CONCAT_TRIM_SET, + CONCAT_WS_SIZE); + element += trim; + elemLength -= trim; + + /* + * Trim away the trailing whitespace. Do not permit trimming to expose + * a final backslash character. + */ - /* Do not permit trimming to expose a final backslash character. */ - elemLength += trimr && (element[elemLength - 1] == '\\'); + trim = TclTrimRight(element, elemLength, CONCAT_TRIM_SET, + CONCAT_WS_SIZE); + trim -= trim && (element[elemLength - trim - 1] == '\\'); + elemLength -= trim; /* * If we're left with empty element after trimming, do nothing. @@ -2133,18 +2012,28 @@ Tcl_ConcatObj( Tcl_SetObjLength(resPtr, 0); for (i = 0; i < objc; i++) { - int triml, trimr; + int trim; element = TclGetStringFromObj(objv[i], &elemLength); - /* Trim away the leading/trailing whitespace. */ - triml = TclTrim(element, elemLength, CONCAT_TRIM_SET, - CONCAT_WS_SIZE, &trimr); - element += triml; - elemLength -= triml + trimr; + /* + * Trim away the leading whitespace. + */ + + trim = TclTrimLeft(element, elemLength, CONCAT_TRIM_SET, + CONCAT_WS_SIZE); + element += trim; + elemLength -= trim; + + /* + * Trim away the trailing whitespace. Do not permit trimming to expose + * a final backslash character. + */ - /* Do not permit trimming to expose a final backslash character. */ - elemLength += trimr && (element[elemLength - 1] == '\\'); + trim = TclTrimRight(element, elemLength, CONCAT_TRIM_SET, + CONCAT_WS_SIZE); + trim -= trim && (element[elemLength - trim - 1] == '\\'); + elemLength -= trim; /* * If we're left with empty element after trimming, do nothing. diff --git a/tests/oo.test b/tests/oo.test index 22e3f11..4f9490b 100644 --- a/tests/oo.test +++ b/tests/oo.test @@ -57,13 +57,7 @@ test oo-0.4 {basic test of OO's ability to clean up its initial state} -body { foo destroy } } -constraints memory -result 0 -test oo-0.5.1 {testing object foundation cleanup} memory { - leaktest { - interp create foo - interp delete foo - } -} 0 -test oo-0.5.2 {testing literal leak on interp delete} memory { +test oo-0.5 {testing literal leak on interp delete} memory { leaktest { interp create foo foo eval {oo::object new} -- cgit v0.12 From 0a3e2630364e089f3b5c35a2935c855cc5c5ed5d Mon Sep 17 00:00:00 2001 From: dgp Date: Wed, 14 Mar 2018 05:41:19 +0000 Subject: cherry pick the desirable part of the merge. --- generic/tclCmdMZ.c | 3 +- generic/tclInt.h | 2 + generic/tclUtil.c | 255 ++++++++++++++++++++++++++++++++++++++--------------- 3 files changed, 186 insertions(+), 74 deletions(-) diff --git a/generic/tclCmdMZ.c b/generic/tclCmdMZ.c index ba96d7c..3c5c5e4 100644 --- a/generic/tclCmdMZ.c +++ b/generic/tclCmdMZ.c @@ -3228,8 +3228,7 @@ StringTrimCmd( } string1 = TclGetStringFromObj(objv[1], &length1); - triml = TclTrimLeft(string1, length1, string2, length2); - trimr = TclTrimRight(string1 + triml, length1 - triml, string2, length2); + triml = TclTrim(string1, length1, string2, length2, &trimr); Tcl_SetObjResult(interp, Tcl_NewStringObj(string1 + triml, length1 - triml - trimr)); diff --git a/generic/tclInt.h b/generic/tclInt.h index e7794c7..57f648c 100644 --- a/generic/tclInt.h +++ b/generic/tclInt.h @@ -3198,6 +3198,8 @@ MODULE_SCOPE void TclSubstParse(Tcl_Interp *interp, const char *bytes, MODULE_SCOPE int TclSubstTokens(Tcl_Interp *interp, Tcl_Token *tokenPtr, int count, int *tokensLeftPtr, int line, int *clNextOuter, const char *outerScript); +MODULE_SCOPE int TclTrim(const char *bytes, int numBytes, + const char *trim, int numTrim, int *trimRight); MODULE_SCOPE int TclTrimLeft(const char *bytes, int numBytes, const char *trim, int numTrim); MODULE_SCOPE int TclTrimRight(const char *bytes, int numBytes, diff --git a/generic/tclUtil.c b/generic/tclUtil.c index d1b81a9..3fbc325 100644 --- a/generic/tclUtil.c +++ b/generic/tclUtil.c @@ -1647,11 +1647,46 @@ Tcl_Backslash( /* *---------------------------------------------------------------------- * - * TclTrimRight -- + * UtfWellFormedEnd -- + * Checks the end of utf string is malformed, if yes - wraps bytes + * to the given buffer (as well-formed NTS string). The buffer + * argument should be initialized by the caller and ready to use. + * + * Results: + * The bytes with well-formed end of the string. * - * Takes two counted strings in the Tcl encoding which must both be null - * terminated. Conceptually trims from the right side of the first string - * all characters found in the second string. + * Side effects: + * Buffer (DString) may be allocated, so must be released. + * + *---------------------------------------------------------------------- + */ + +static inline const char* +UtfWellFormedEnd( + Tcl_DString *buffer, /* Buffer used to hold well-formed string. */ + const char *bytes, /* Pointer to the beginning of the string. */ + int length) /* Length of the string. */ +{ + const char *l = bytes + length; + const char *p = Tcl_UtfPrev(l, bytes); + + if (Tcl_UtfCharComplete(p, l - p)) { + return bytes; + } + /* + * Malformed utf-8 end, be sure we've NTS to safe compare of end-character, + * avoid segfault by access violation out of range. + */ + Tcl_DStringAppend(buffer, bytes, length); + return Tcl_DStringValue(buffer); +} +/* + *---------------------------------------------------------------------- + * + * TclTrimRight -- + * Takes two counted strings in the Tcl encoding. Conceptually + * finds the sub string (offset) to trim from the right side of the + * first string all characters found in the second string. * * Results: * The number of bytes to be removed from the end of the string. @@ -1662,8 +1697,8 @@ Tcl_Backslash( *---------------------------------------------------------------------- */ -int -TclTrimRight( +static inline int +TrimRight( const char *bytes, /* String to be trimmed... */ int numBytes, /* ...and its length in bytes */ const char *trim, /* String of trim characters... */ @@ -1673,18 +1708,6 @@ TclTrimRight( int pInc; Tcl_UniChar ch1 = 0, ch2 = 0; - if ((bytes[numBytes] != '\0') || (trim[numTrim] != '\0')) { - Tcl_Panic("TclTrimRight works only on null-terminated strings"); - } - - /* - * Empty strings -> nothing to do. - */ - - if ((numBytes == 0) || (numTrim == 0)) { - return 0; - } - /* * Outer loop: iterate over string to be trimmed. */ @@ -1723,15 +1746,46 @@ TclTrimRight( return numBytes - (p - bytes); } + +int +TclTrimRight( + const char *bytes, /* String to be trimmed... */ + int numBytes, /* ...and its length in bytes */ + const char *trim, /* String of trim characters... */ + int numTrim) /* ...and its length in bytes */ +{ + int res; + Tcl_DString bytesBuf, trimBuf; + + /* Empty strings -> nothing to do */ + if ((numBytes == 0) || (numTrim == 0)) { + return 0; + } + + Tcl_DStringInit(&bytesBuf); + Tcl_DStringInit(&trimBuf); + bytes = UtfWellFormedEnd(&bytesBuf, bytes, numBytes); + trim = UtfWellFormedEnd(&trimBuf, trim, numTrim); + + res = TrimRight(bytes, numBytes, trim, numTrim); + if (res > numBytes) { + res = numBytes; + } + + Tcl_DStringFree(&bytesBuf); + Tcl_DStringFree(&trimBuf); + + return res; +} /* *---------------------------------------------------------------------- * * TclTrimLeft -- * - * Takes two counted strings in the Tcl encoding which must both be null - * terminated. Conceptually trims from the left side of the first string - * all characters found in the second string. + * Takes two counted strings in the Tcl encoding. Conceptually + * finds the sub string (offset) to trim from the left side of the + * first string all characters found in the second string. * * Results: * The number of bytes to be removed from the start of the string. @@ -1742,8 +1796,8 @@ TclTrimRight( *---------------------------------------------------------------------- */ -int -TclTrimLeft( +static inline int +TrimLeft( const char *bytes, /* String to be trimmed... */ int numBytes, /* ...and its length in bytes */ const char *trim, /* String of trim characters... */ @@ -1752,18 +1806,6 @@ TclTrimLeft( const char *p = bytes; Tcl_UniChar ch1 = 0, ch2 = 0; - if ((bytes[numBytes] != '\0') || (trim[numTrim] != '\0')) { - Tcl_Panic("TclTrimLeft works only on null-terminated strings"); - } - - /* - * Empty strings -> nothing to do. - */ - - if ((numBytes == 0) || (numTrim == 0)) { - return 0; - } - /* * Outer loop: iterate over string to be trimmed. */ @@ -1798,10 +1840,99 @@ TclTrimLeft( p += pInc; numBytes -= pInc; - } while (numBytes); + } while (numBytes > 0); return p - bytes; } + +int +TclTrimLeft( + const char *bytes, /* String to be trimmed... */ + int numBytes, /* ...and its length in bytes */ + const char *trim, /* String of trim characters... */ + int numTrim) /* ...and its length in bytes */ +{ + int res; + Tcl_DString bytesBuf, trimBuf; + + /* Empty strings -> nothing to do */ + if ((numBytes == 0) || (numTrim == 0)) { + return 0; + } + + Tcl_DStringInit(&bytesBuf); + Tcl_DStringInit(&trimBuf); + bytes = UtfWellFormedEnd(&bytesBuf, bytes, numBytes); + trim = UtfWellFormedEnd(&trimBuf, trim, numTrim); + + res = TrimLeft(bytes, numBytes, trim, numTrim); + if (res > numBytes) { + res = numBytes; + } + + Tcl_DStringFree(&bytesBuf); + Tcl_DStringFree(&trimBuf); + + return res; +} + +/* + *---------------------------------------------------------------------- + * + * TclTrim -- + * Finds the sub string (offset) to trim from both sides of the + * first string all characters found in the second string. + * + * Results: + * The number of bytes to be removed from the start of the string + * + * Side effects: + * None. + * + *---------------------------------------------------------------------- + */ + +int +TclTrim( + const char *bytes, /* String to be trimmed... */ + int numBytes, /* ...and its length in bytes */ + const char *trim, /* String of trim characters... */ + int numTrim, /* ...and its length in bytes */ + int *trimRight) /* Offset from the end of the string. */ +{ + int trimLeft; + Tcl_DString bytesBuf, trimBuf; + + /* Empty strings -> nothing to do */ + if ((numBytes == 0) || (numTrim == 0)) { + *trimRight = 0; + return 0; + } + + Tcl_DStringInit(&bytesBuf); + Tcl_DStringInit(&trimBuf); + bytes = UtfWellFormedEnd(&bytesBuf, bytes, numBytes); + trim = UtfWellFormedEnd(&trimBuf, trim, numTrim); + + trimLeft = TrimLeft(bytes, numBytes, trim, numTrim); + if (trimLeft > numBytes) { + trimLeft = numBytes; + } + numBytes -= trimLeft; + *trimRight = 0; + if (numBytes) { + bytes += trimLeft; + *trimRight = TrimRight(bytes, numBytes, trim, numTrim); + if (*trimRight > numBytes) { + *trimRight = numBytes; + } + } + + Tcl_DStringFree(&bytesBuf); + Tcl_DStringFree(&trimBuf); + + return trimLeft; +} /* *---------------------------------------------------------------------- @@ -1869,30 +2000,20 @@ Tcl_Concat( result = ckalloc((unsigned) (bytesNeeded + argc)); for (p = result, i = 0; i < argc; i++) { - int trim, elemLength; + int triml, trimr, elemLength; const char *element; element = argv[i]; elemLength = strlen(argv[i]); - /* - * Trim away the leading whitespace. - */ - - trim = TclTrimLeft(element, elemLength, CONCAT_TRIM_SET, - CONCAT_WS_SIZE); - element += trim; - elemLength -= trim; - - /* - * Trim away the trailing whitespace. Do not permit trimming to expose - * a final backslash character. - */ + /* Trim away the leading/trailing whitespace. */ + triml = TclTrim(element, elemLength, CONCAT_TRIM_SET, + CONCAT_WS_SIZE, &trimr); + element += triml; + elemLength -= triml + trimr; - trim = TclTrimRight(element, elemLength, CONCAT_TRIM_SET, - CONCAT_WS_SIZE); - trim -= trim && (element[elemLength - trim - 1] == '\\'); - elemLength -= trim; + /* Do not permit trimming to expose a final backslash character. */ + elemLength += trimr && (element[elemLength - 1] == '\\'); /* * If we're left with empty element after trimming, do nothing. @@ -2012,28 +2133,18 @@ Tcl_ConcatObj( Tcl_SetObjLength(resPtr, 0); for (i = 0; i < objc; i++) { - int trim; + int triml, trimr; element = TclGetStringFromObj(objv[i], &elemLength); - /* - * Trim away the leading whitespace. - */ - - trim = TclTrimLeft(element, elemLength, CONCAT_TRIM_SET, - CONCAT_WS_SIZE); - element += trim; - elemLength -= trim; - - /* - * Trim away the trailing whitespace. Do not permit trimming to expose - * a final backslash character. - */ + /* Trim away the leading/trailing whitespace. */ + triml = TclTrim(element, elemLength, CONCAT_TRIM_SET, + CONCAT_WS_SIZE, &trimr); + element += triml; + elemLength -= triml + trimr; - trim = TclTrimRight(element, elemLength, CONCAT_TRIM_SET, - CONCAT_WS_SIZE); - trim -= trim && (element[elemLength - trim - 1] == '\\'); - elemLength -= trim; + /* Do not permit trimming to expose a final backslash character. */ + elemLength += trimr && (element[elemLength - 1] == '\\'); /* * If we're left with empty element after trimming, do nothing. -- cgit v0.12