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