From 6740ac50e9a65342cacd77245cacb8ad443dda00 Mon Sep 17 00:00:00 2001 From: dgp Date: Fri, 27 Jan 2012 21:56:52 +0000 Subject: 3479689 New internal routine TclJoinPath(). Refactor all the *Join*Path* routines to give them more useful interfaces that are easier to manage getting the refcounts right. --- ChangeLog | 8 +++++++ generic/tclCmdAH.c | 2 +- generic/tclFCmd.c | 5 +---- generic/tclFileName.c | 40 +++++++++++++++------------------ generic/tclInt.h | 1 + generic/tclPathObj.c | 61 ++++++++++++++++++++++----------------------------- 6 files changed, 55 insertions(+), 62 deletions(-) diff --git a/ChangeLog b/ChangeLog index 896f9a0..e43d7ac 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,13 @@ 2012-01-26 Don Porter + * generic/tclCmdAH.c: New internal routine TclJoinPath(). + * generic/tclFCmd.c: Refactor all the *Join*Path* routines + * generic/tclFileName.c: to give them more useful interfaces + * generic/tclInt.h: that are easier to manage getting the + * generic/tclPathObj.c: refcounts right. [Bug 3479689] + +2012-01-26 Don Porter + * generic/tclPathObj.c: [Bug 3475569]: Add checks for unshared values before calls demanding them. [Bug 3479689]: Stop memory corruption when shimmering 0-refCount value to "path" type. diff --git a/generic/tclCmdAH.c b/generic/tclCmdAH.c index d036bd6..2308f33 100644 --- a/generic/tclCmdAH.c +++ b/generic/tclCmdAH.c @@ -1841,7 +1841,7 @@ PathJoinCmd( Tcl_WrongNumArgs(interp, 1, objv, "name ?name ...?"); return TCL_ERROR; } - Tcl_SetObjResult(interp, Tcl_FSJoinToPath(NULL, objc - 1, objv + 1)); + Tcl_SetObjResult(interp, TclJoinPath(objc - 1, objv + 1)); return TCL_OK; } diff --git a/generic/tclFCmd.c b/generic/tclFCmd.c index 0d90094..e95a136 100644 --- a/generic/tclFCmd.c +++ b/generic/tclFCmd.c @@ -177,7 +177,6 @@ FileCopyRename( for ( ; irefCount--; - return ret; + pair[0] = pathPtr; + pair[1] = objv[0]; + return TclJoinPath(2, pair); + } else { + int elemc = objc + 1; + Tcl_Obj *ret, **elemv = ckalloc(elemc*sizeof(Tcl_Obj **)); + + elemv[0] = pathPtr; + memcpy(elemv+1, objv, objc*sizeof(Tcl_Obj **)); + ret = TclJoinPath(elemc, elemv); + ckfree(elemv); + return ret; + } } /* diff --git a/generic/tclInt.h b/generic/tclInt.h index b375bb9..feede54 100644 --- a/generic/tclInt.h +++ b/generic/tclInt.h @@ -2995,6 +2995,7 @@ MODULE_SCOPE void TclInitSubsystems(void); MODULE_SCOPE int TclInterpReady(Tcl_Interp *interp); MODULE_SCOPE int TclIsLocalScalar(const char *src, int len); MODULE_SCOPE int TclIsSpaceProc(char byte); +MODULE_SCOPE Tcl_Obj * TclJoinPath(int elements, Tcl_Obj * const objv[]); MODULE_SCOPE int TclJoinThread(Tcl_ThreadId id, int *result); MODULE_SCOPE void TclLimitRemoveAllHandlers(Tcl_Interp *interp); MODULE_SCOPE Tcl_Obj * TclLindexList(Tcl_Interp *interp, diff --git a/generic/tclPathObj.c b/generic/tclPathObj.c index 30f2081..7ab8a4e 100644 --- a/generic/tclPathObj.c +++ b/generic/tclPathObj.c @@ -838,44 +838,39 @@ Tcl_FSJoinPath( * reference count. */ int elements) /* Number of elements to use (-1 = all) */ { - Tcl_Obj *res; - int i; - const Tcl_Filesystem *fsPtr = NULL; + Tcl_Obj *copy, *res; + int objc; + Tcl_Obj **objv; - if (elements < 0) { - if (Tcl_ListObjLength(NULL, listObj, &elements) != TCL_OK) { - return NULL; - } - } else { - /* - * Just make sure it is a valid list. - */ - - int listTest; - - if (Tcl_ListObjLength(NULL, listObj, &listTest) != TCL_OK) { - return NULL; - } + if (Tcl_ListObjLength(NULL, listObj, &objc) != TCL_OK) { + return NULL; + } - /* - * Correct this if it is too large, otherwise we will waste our time - * joining null elements to the path. - */ + elements = ((elements >= 0) && (elements <= objc)) ? elements : objc; + copy = TclListObjCopy(NULL, listObj); + Tcl_ListObjGetElements(NULL, listObj, &objc, &objv); + res = TclJoinPath(elements, objv); + Tcl_DecrRefCount(copy); + return res; +} - if (elements > listTest) { - elements = listTest; - } - } +Tcl_Obj * +TclJoinPath( + int elements, + Tcl_Obj * const objv[]) +{ + Tcl_Obj *res; + int i; + const Tcl_Filesystem *fsPtr = NULL; res = NULL; for (i = 0; i < elements; i++) { - Tcl_Obj *elt, *driveName = NULL; int driveNameLength, strEltLen, length; Tcl_PathType type; char *strElt, *ptr; - - Tcl_ListObjIndex(NULL, listObj, i, &elt); + Tcl_Obj *driveName = NULL; + Tcl_Obj *elt = objv[i]; /* * This is a special case where we can be much more efficient, where @@ -889,9 +884,8 @@ Tcl_FSJoinPath( if ((i == (elements-2)) && (i == 0) && (elt->typePtr == &tclFsPathType) && !((elt->bytes != NULL) && (elt->bytes[0] == '\0'))) { - Tcl_Obj *tailObj; + Tcl_Obj *tailObj = objv[i+1]; - Tcl_ListObjIndex(NULL, listObj, i+1, &tailObj); type = TclGetPathType(tailObj, NULL, NULL, NULL); if (type == TCL_PATH_RELATIVE) { const char *str; @@ -1389,7 +1383,7 @@ AppendPath( * of no evidence that such a foolish thing exists. This solution was * chosen so that "JoinPath" operations that pass through either path * intrep produce the same results; that is, bugward compatibility. If - * we need to fix that bug here, it needs fixing in Tcl_FSJoinPath() too. + * we need to fix that bug here, it needs fixing in TclJoinPath() too. */ bytes = Tcl_GetStringFromObj(tail, &numBytes); if (numBytes == 0) { @@ -2499,10 +2493,7 @@ SetFsPathFromAny( } Tcl_DStringFree(&temp); } else { - /* Bug 3479689: protect 0-refcount pathPth from getting freed */ - pathPtr->refCount++; - transPtr = Tcl_FSJoinToPath(pathPtr, 0, NULL); - pathPtr->refCount--; + transPtr = TclJoinPath(1, &pathPtr); } #if defined(__CYGWIN__) && defined(__WIN32__) -- cgit v0.12