From 8f25f4915310d084a6d0e973b8aa85ba18d7bee7 Mon Sep 17 00:00:00 2001 From: dgp Date: Thu, 6 Jul 2017 15:46:58 +0000 Subject: Alternative fix for memleaks in fs path join machinery. --- generic/tclPathObj.c | 33 ++++++++++++++++----------------- 1 file changed, 16 insertions(+), 17 deletions(-) diff --git a/generic/tclPathObj.c b/generic/tclPathObj.c index 31ed68e..f8015b2 100644 --- a/generic/tclPathObj.c +++ b/generic/tclPathObj.c @@ -13,6 +13,7 @@ #include "tclInt.h" #include "tclFileSystem.h" +#include /* * Prototypes for functions defined later in this file. @@ -849,11 +850,17 @@ TclJoinPath( int elements, Tcl_Obj * const objv[]) { - Tcl_Obj *res; + Tcl_Obj *res = NULL; int i; const Tcl_Filesystem *fsPtr = NULL; - res = NULL; + assert ( elements >= 0 ); + + if (elements == 0) { + return Tcl_NewObj(); + } + + assert ( elements > 0 ); for (i = 0; i < elements; i++) { int driveNameLength, strEltLen, length; @@ -893,9 +900,7 @@ TclJoinPath( * the base itself is just fine! */ - if (res != NULL) { - TclDecrRefCount(res); - } + assert ( res == NULL ); return elt; } @@ -918,9 +923,8 @@ TclJoinPath( if ((tclPlatform != TCL_PLATFORM_WINDOWS) || (strchr(Tcl_GetString(elt), '\\') == NULL)) { - if (res != NULL) { - TclDecrRefCount(res); - } + + assert ( res == NULL ); if (PATHFLAGS(elt)) { return TclNewFSPathObj(elt, str, len); @@ -940,18 +944,14 @@ TclJoinPath( * more general code below handle things. */ } else if (tclPlatform == TCL_PLATFORM_UNIX) { - if (res != NULL) { - TclDecrRefCount(res); - } + assert ( res == NULL ); return tailObj; } else { const char *str = TclGetString(tailObj); if (tclPlatform == TCL_PLATFORM_WINDOWS) { if (strchr(str, '\\') == NULL) { - if (res != NULL) { - TclDecrRefCount(res); - } + assert ( res == NULL ); return tailObj; } } @@ -1087,6 +1087,7 @@ TclJoinPath( if (sep != NULL) { separator = TclGetString(sep)[0]; + Tcl_DecrRefCount(sep); } /* Safety check in case the VFS driver caused sharing */ if (Tcl_IsShared(res)) { @@ -1122,9 +1123,7 @@ TclJoinPath( Tcl_SetObjLength(res, length); } } - if (res == NULL) { - res = Tcl_NewObj(); - } + assert ( res != NULL ); return res; } -- cgit v0.12 From 5ef31506293557a4127815e05758bedf889f9b5b Mon Sep 17 00:00:00 2001 From: dgp Date: Thu, 6 Jul 2017 16:00:46 +0000 Subject: Pull out of the loop a block of code that can only run in first iteration. --- generic/tclPathObj.c | 37 +++++++++++++++++++------------------ 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/generic/tclPathObj.c b/generic/tclPathObj.c index f8015b2..8ee4869 100644 --- a/generic/tclPathObj.c +++ b/generic/tclPathObj.c @@ -862,12 +862,8 @@ TclJoinPath( assert ( elements > 0 ); - for (i = 0; i < elements; i++) { - int driveNameLength, strEltLen, length; - Tcl_PathType type; - char *strElt, *ptr; - Tcl_Obj *driveName = NULL; - Tcl_Obj *elt = objv[i]; + if (elements == 2) { + Tcl_Obj *elt = objv[0]; /* * This is a special case where we can be much more efficient, where @@ -876,18 +872,17 @@ TclJoinPath( * object which can be normalized more efficiently. Currently we only * use the special case when we have exactly two elements, but we * could expand that in the future. - * - * Bugfix [a47641a0]. TclNewFSPathObj requires first argument - * to be an absolute path. Added a check for that elt is absolute. + * + * Bugfix [a47641a0]. TclNewFSPathObj requires first argument + * to be an absolute path. Added a check for that elt is absolute. */ - if ((i == (elements-2)) && (i == 0) - && (elt->typePtr == &tclFsPathType) + if ((elt->typePtr == &tclFsPathType) && !((elt->bytes != NULL) && (elt->bytes[0] == '\0')) && TclGetPathType(elt, NULL, NULL, NULL) == TCL_PATH_ABSOLUTE) { - Tcl_Obj *tailObj = objv[i+1]; + Tcl_Obj *tailObj = objv[1]; + Tcl_PathType type = TclGetPathType(tailObj, NULL, NULL, NULL); - type = TclGetPathType(tailObj, NULL, NULL, NULL); if (type == TCL_PATH_RELATIVE) { const char *str; int len; @@ -900,7 +895,6 @@ TclJoinPath( * the base itself is just fine! */ - assert ( res == NULL ); return elt; } @@ -924,8 +918,6 @@ TclJoinPath( if ((tclPlatform != TCL_PLATFORM_WINDOWS) || (strchr(Tcl_GetString(elt), '\\') == NULL)) { - assert ( res == NULL ); - if (PATHFLAGS(elt)) { return TclNewFSPathObj(elt, str, len); } @@ -944,19 +936,28 @@ TclJoinPath( * more general code below handle things. */ } else if (tclPlatform == TCL_PLATFORM_UNIX) { - assert ( res == NULL ); return tailObj; } else { const char *str = TclGetString(tailObj); if (tclPlatform == TCL_PLATFORM_WINDOWS) { if (strchr(str, '\\') == NULL) { - assert ( res == NULL ); return tailObj; } } } } + } + + assert ( res == NULL ); + + for (i = 0; i < elements; i++) { + int driveNameLength, strEltLen, length; + Tcl_PathType type; + char *strElt, *ptr; + Tcl_Obj *driveName = NULL; + Tcl_Obj *elt = objv[i]; + strElt = Tcl_GetStringFromObj(elt, &strEltLen); driveNameLength = 0; type = TclGetPathType(elt, &fsPtr, &driveNameLength, &driveName); -- cgit v0.12 From ba36e5644b01038e11624290850803281b18ece1 Mon Sep 17 00:00:00 2001 From: "jan.nijtmans" Date: Thu, 13 Jul 2017 15:03:27 +0000 Subject: Fix [293344d4f3]: Regression in SQLite test-suite. Long-standing bug in implementation of TclUtfToUniChar() macro. --- generic/tclInt.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/generic/tclInt.h b/generic/tclInt.h index 6113f23..25bec6a 100644 --- a/generic/tclInt.h +++ b/generic/tclInt.h @@ -3670,7 +3670,7 @@ MODULE_SCOPE void TclDbInitNewObj(Tcl_Obj *objPtr, CONST char *file, #define TclUtfToUniChar(str, chPtr) \ ((((unsigned char) *(str)) < 0xC0) ? \ - ((*(chPtr) = (Tcl_UniChar) *(str)), 1) \ + ((*(chPtr) = (unsigned char) *(str)), 1) \ : Tcl_UtfToUniChar(str, chPtr)) /* -- cgit v0.12