summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authordkf <donal.k.fellows@manchester.ac.uk>2014-06-15 07:52:57 (GMT)
committerdkf <donal.k.fellows@manchester.ac.uk>2014-06-15 07:52:57 (GMT)
commit92590b85097cd3682ec43080549d54bf7236ca76 (patch)
treef6aa4d2ba95d16bc1d7a0cc589e5e9581dcda22c
parent860a7252bb242157af32c222cca494d3d9635bc4 (diff)
downloadtcl-92590b85097cd3682ec43080549d54bf7236ca76.zip
tcl-92590b85097cd3682ec43080549d54bf7236ca76.tar.gz
tcl-92590b85097cd3682ec43080549d54bf7236ca76.tar.bz2
Make [dict replace] and [dict remove] guarantee result canonicality.
-rw-r--r--generic/tclDictObj.c149
-rw-r--r--tests/dict.test30
2 files changed, 84 insertions, 95 deletions
diff --git a/generic/tclDictObj.c b/generic/tclDictObj.c
index 77f66fb..a057b8a 100644
--- a/generic/tclDictObj.c
+++ b/generic/tclDictObj.c
@@ -770,10 +770,9 @@ TclTraceDictPath(
Dict *dict, *newDict;
int i;
- if (dictPtr->typePtr != &tclDictType) {
- if (SetDictFromAny(interp, dictPtr) != TCL_OK) {
- return NULL;
- }
+ if (dictPtr->typePtr != &tclDictType
+ && SetDictFromAny(interp, dictPtr) != TCL_OK) {
+ return NULL;
}
dict = dictPtr->internalRep.twoPtrValue.ptr1;
if (flags & DICT_PATH_UPDATE) {
@@ -811,10 +810,9 @@ TclTraceDictPath(
Tcl_SetHashValue(hPtr, tmpObj);
} else {
tmpObj = Tcl_GetHashValue(hPtr);
- if (tmpObj->typePtr != &tclDictType) {
- if (SetDictFromAny(interp, tmpObj) != TCL_OK) {
- return NULL;
- }
+ if (tmpObj->typePtr != &tclDictType
+ && SetDictFromAny(interp, tmpObj) != TCL_OK) {
+ return NULL;
}
}
@@ -909,12 +907,9 @@ Tcl_DictObjPut(
Tcl_Panic("%s called with shared object", "Tcl_DictObjPut");
}
- if (dictPtr->typePtr != &tclDictType) {
- int result = SetDictFromAny(interp, dictPtr);
-
- if (result != TCL_OK) {
- return result;
- }
+ if (dictPtr->typePtr != &tclDictType
+ && SetDictFromAny(interp, dictPtr) != TCL_OK) {
+ return TCL_ERROR;
}
if (dictPtr->bytes != NULL) {
@@ -963,12 +958,10 @@ Tcl_DictObjGet(
Dict *dict;
Tcl_HashEntry *hPtr;
- if (dictPtr->typePtr != &tclDictType) {
- int result = SetDictFromAny(interp, dictPtr);
- if (result != TCL_OK) {
- *valuePtrPtr = NULL;
- return result;
- }
+ if (dictPtr->typePtr != &tclDictType
+ && SetDictFromAny(interp, dictPtr) != TCL_OK) {
+ *valuePtrPtr = NULL;
+ return TCL_ERROR;
}
dict = dictPtr->internalRep.twoPtrValue.ptr1;
@@ -1012,11 +1005,9 @@ Tcl_DictObjRemove(
Tcl_Panic("%s called with shared object", "Tcl_DictObjRemove");
}
- if (dictPtr->typePtr != &tclDictType) {
- int result = SetDictFromAny(interp, dictPtr);
- if (result != TCL_OK) {
- return result;
- }
+ if (dictPtr->typePtr != &tclDictType
+ && SetDictFromAny(interp, dictPtr) != TCL_OK) {
+ return TCL_ERROR;
}
dict = dictPtr->internalRep.twoPtrValue.ptr1;
@@ -1055,11 +1046,9 @@ Tcl_DictObjSize(
{
Dict *dict;
- if (dictPtr->typePtr != &tclDictType) {
- int result = SetDictFromAny(interp, dictPtr);
- if (result != TCL_OK) {
- return result;
- }
+ if (dictPtr->typePtr != &tclDictType
+ && SetDictFromAny(interp, dictPtr) != TCL_OK) {
+ return TCL_ERROR;
}
dict = dictPtr->internalRep.twoPtrValue.ptr1;
@@ -1109,12 +1098,9 @@ Tcl_DictObjFirst(
Dict *dict;
ChainEntry *cPtr;
- if (dictPtr->typePtr != &tclDictType) {
- int result = SetDictFromAny(interp, dictPtr);
-
- if (result != TCL_OK) {
- return result;
- }
+ if (dictPtr->typePtr != &tclDictType
+ && SetDictFromAny(interp, dictPtr) != TCL_OK) {
+ return TCL_ERROR;
}
dict = dictPtr->internalRep.twoPtrValue.ptr1;
@@ -1497,7 +1483,7 @@ DictCreateCmd(
/*
* The next command is assumed to never fail...
*/
- Tcl_DictObjPut(interp, dictObj, objv[i], objv[i+1]);
+ Tcl_DictObjPut(NULL, dictObj, objv[i], objv[i+1]);
}
Tcl_SetObjResult(interp, dictObj);
return TCL_OK;
@@ -1630,17 +1616,18 @@ DictReplaceCmd(
}
dictPtr = objv[1];
- if (dictPtr->typePtr != &tclDictType) {
- int result = SetDictFromAny(interp, dictPtr);
- if (result != TCL_OK) {
- return result;
- }
+ if (dictPtr->typePtr != &tclDictType
+ && SetDictFromAny(interp, dictPtr) != TCL_OK) {
+ return TCL_ERROR;
+ }
+ if (Tcl_IsShared(dictPtr)) {
+ dictPtr = Tcl_DuplicateObj(dictPtr);
+ }
+ if (dictPtr->bytes != NULL) {
+ TclInvalidateStringRep(dictPtr);
}
for (i=2 ; i<objc ; i+=2) {
- if (Tcl_IsShared(dictPtr)) {
- dictPtr = Tcl_DuplicateObj(dictPtr);
- }
- (void) Tcl_DictObjPut(interp, dictPtr, objv[i], objv[i+1]);
+ Tcl_DictObjPut(NULL, dictPtr, objv[i], objv[i+1]);
}
Tcl_SetObjResult(interp, dictPtr);
return TCL_OK;
@@ -1680,17 +1667,18 @@ DictRemoveCmd(
}
dictPtr = objv[1];
- if (dictPtr->typePtr != &tclDictType) {
- int result = SetDictFromAny(interp, dictPtr);
- if (result != TCL_OK) {
- return result;
- }
+ if (dictPtr->typePtr != &tclDictType
+ && SetDictFromAny(interp, dictPtr) != TCL_OK) {
+ return TCL_ERROR;
+ }
+ if (Tcl_IsShared(dictPtr)) {
+ dictPtr = Tcl_DuplicateObj(dictPtr);
+ }
+ if (dictPtr->bytes != NULL) {
+ TclInvalidateStringRep(dictPtr);
}
for (i=2 ; i<objc ; i++) {
- if (Tcl_IsShared(dictPtr)) {
- dictPtr = Tcl_DuplicateObj(dictPtr);
- }
- (void) Tcl_DictObjRemove(interp, dictPtr, objv[i]);
+ Tcl_DictObjRemove(NULL, dictPtr, objv[i]);
}
Tcl_SetObjResult(interp, dictPtr);
return TCL_OK;
@@ -1739,10 +1727,9 @@ DictMergeCmd(
*/
targetObj = objv[1];
- if (targetObj->typePtr != &tclDictType) {
- if (SetDictFromAny(interp, targetObj) != TCL_OK) {
- return TCL_ERROR;
- }
+ if (targetObj->typePtr != &tclDictType
+ && SetDictFromAny(interp, targetObj) != TCL_OK) {
+ return TCL_ERROR;
}
if (objc == 2) {
@@ -1824,12 +1811,9 @@ DictKeysCmd(
* need. [Bug 1705778, leak K04]
*/
- if (objv[1]->typePtr != &tclDictType) {
- int result = SetDictFromAny(interp, objv[1]);
-
- if (result != TCL_OK) {
- return result;
- }
+ if (objv[1]->typePtr != &tclDictType
+ && SetDictFromAny(interp, objv[1]) != TCL_OK) {
+ return TCL_ERROR;
}
if (objc == 3) {
@@ -2045,11 +2029,9 @@ DictInfoCmd(
}
dictPtr = objv[1];
- if (dictPtr->typePtr != &tclDictType) {
- int result = SetDictFromAny(interp, dictPtr);
- if (result != TCL_OK) {
- return result;
- }
+ if (dictPtr->typePtr != &tclDictType
+ && SetDictFromAny(interp, dictPtr) != TCL_OK) {
+ return TCL_ERROR;
}
dict = dictPtr->internalRep.twoPtrValue.ptr1;
@@ -2141,10 +2123,10 @@ DictIncrCmd(
*/
mp_clear(&increment);
- Tcl_DictObjPut(interp, dictPtr, objv[2], objv[3]);
+ Tcl_DictObjPut(NULL, dictPtr, objv[2], objv[3]);
}
} else {
- Tcl_DictObjPut(interp, dictPtr, objv[2], Tcl_NewIntObj(1));
+ Tcl_DictObjPut(NULL, dictPtr, objv[2], Tcl_NewIntObj(1));
}
} else {
/*
@@ -2153,7 +2135,7 @@ DictIncrCmd(
if (Tcl_IsShared(valuePtr)) {
valuePtr = Tcl_DuplicateObj(valuePtr);
- Tcl_DictObjPut(interp, dictPtr, objv[2], valuePtr);
+ Tcl_DictObjPut(NULL, dictPtr, objv[2], valuePtr);
}
if (objc == 4) {
code = TclIncrObj(interp, valuePtr, objv[3]);
@@ -2253,7 +2235,7 @@ DictLappendCmd(
}
if (allocatedValue) {
- Tcl_DictObjPut(interp, dictPtr, objv[2], valuePtr);
+ Tcl_DictObjPut(NULL, dictPtr, objv[2], valuePtr);
} else if (dictPtr->bytes != NULL) {
TclInvalidateStringRep(dictPtr);
}
@@ -2328,7 +2310,7 @@ DictAppendCmd(
Tcl_AppendObjToObj(valuePtr, objv[i]);
}
- Tcl_DictObjPut(interp, dictPtr, objv[2], valuePtr);
+ Tcl_DictObjPut(NULL, dictPtr, objv[2], valuePtr);
resultPtr = Tcl_ObjSetVar2(interp, objv[1], NULL, dictPtr,
TCL_LEAVE_ERR_MSG);
@@ -2938,12 +2920,12 @@ DictFilterCmd(
Tcl_DictObjDone(&search);
Tcl_DictObjGet(interp, objv[1], objv[3], &valueObj);
if (valueObj != NULL) {
- Tcl_DictObjPut(interp, resultObj, objv[3], valueObj);
+ Tcl_DictObjPut(NULL, resultObj, objv[3], valueObj);
}
} else {
while (!done) {
if (Tcl_StringMatch(TclGetString(keyObj), pattern)) {
- Tcl_DictObjPut(interp, resultObj, keyObj, valueObj);
+ Tcl_DictObjPut(NULL, resultObj, keyObj, valueObj);
}
Tcl_DictObjNext(&search, &keyObj, &valueObj, &done);
}
@@ -2961,7 +2943,7 @@ DictFilterCmd(
for (i=3 ; i<objc ; i++) {
pattern = TclGetString(objv[i]);
if (Tcl_StringMatch(TclGetString(keyObj), pattern)) {
- Tcl_DictObjPut(interp, resultObj, keyObj, valueObj);
+ Tcl_DictObjPut(NULL, resultObj, keyObj, valueObj);
break; /* stop inner loop */
}
}
@@ -2987,7 +2969,7 @@ DictFilterCmd(
for (i=3 ; i<objc ; i++) {
pattern = TclGetString(objv[i]);
if (Tcl_StringMatch(TclGetString(valueObj), pattern)) {
- Tcl_DictObjPut(interp, resultObj, keyObj, valueObj);
+ Tcl_DictObjPut(NULL, resultObj, keyObj, valueObj);
break; /* stop inner loop */
}
}
@@ -3088,7 +3070,7 @@ DictFilterCmd(
}
TclDecrRefCount(boolObj);
if (satisfied) {
- Tcl_DictObjPut(interp, resultObj, keyObj, valueObj);
+ Tcl_DictObjPut(NULL, resultObj, keyObj, valueObj);
}
break;
case TCL_BREAK:
@@ -3276,18 +3258,17 @@ FinalizeDictUpdate(
for (i=0 ; i<objc ; i+=2) {
objPtr = Tcl_ObjGetVar2(interp, objv[i+1], NULL, 0);
if (objPtr == NULL) {
- Tcl_DictObjRemove(interp, dictPtr, objv[i]);
+ Tcl_DictObjRemove(NULL, dictPtr, objv[i]);
} else if (objPtr == dictPtr) {
/*
* Someone is messing us around, trying to build a recursive
* structure. [Bug 1786481]
*/
- Tcl_DictObjPut(interp, dictPtr, objv[i],
- Tcl_DuplicateObj(objPtr));
+ Tcl_DictObjPut(NULL, dictPtr, objv[i], Tcl_DuplicateObj(objPtr));
} else {
/* Shouldn't fail */
- Tcl_DictObjPut(interp, dictPtr, objv[i], objPtr);
+ Tcl_DictObjPut(NULL, dictPtr, objv[i], objPtr);
}
}
TclDecrRefCount(argsObj);
diff --git a/tests/dict.test b/tests/dict.test
index 1439af9..642abd8 100644
--- a/tests/dict.test
+++ b/tests/dict.test
@@ -167,12 +167,12 @@ test dict-4.8 {dict replace command} -returnCodes error -body {
} -result {missing value to go with key}
test dict-4.9 {dict replace command} {dict replace [list a a] a b} {a b}
test dict-4.10 {dict replace command} {dict replace [list a a] a b a c} {a c}
-test dict-4.11 {dict replace command: canonicality not forced} {
- dict replace { a b c d }
-} { a b c d }
-test dict-4.12 {dict replace command: canonicality forced by update} {
- dict replace { a b c d } a b
+test dict-4.11 {dict replace command: canonicality is forced} {
+ dict replace { a b c d }
} {a b c d}
+test dict-4.12 {dict replace command: canonicality is forced} {
+ dict replace {a b c d a e}
+} {a e c d}
test dict-4.13 {dict replace command: type check is mandatory} -body {
dict replace { a b c d e }
} -returnCodes error -result {missing value to go with key}
@@ -208,6 +208,10 @@ test dict-4.17a {dict replace command: type check is mandatory} {
catch {dict replace " a b \{c d "} -> opt
dict get $opt -errorcode
} {TCL VALUE DICTIONARY BRACE}
+test dict-4.18 {dict replace command: canonicality forcing doesn't leak} {
+ set example { a b c d }
+ list $example [dict replace $example]
+} {{ a b c d } {a b c d}}
test dict-5.1 {dict remove command} {dict remove {a b c d} a} {c d}
test dict-5.2 {dict remove command} {dict remove {a b c d} c} {a b}
@@ -220,12 +224,12 @@ test dict-5.6 {dict remove command} {dict remove {a b} c} {a b}
test dict-5.7 {dict remove command} -returnCodes error -body {
dict remove
} -result {wrong # args: should be "dict remove dictionary ?key ...?"}
-test dict-5.8 {dict remove command: canonicality not forced} {
- dict remove { a b c d }
-} { a b c d }
-test dict-5.9 {dict remove command: canonicality not forced} {
- dict remove { a b c d } e
-} { a b c d }
+test dict-5.8 {dict remove command: canonicality is forced} {
+ dict remove { a b c d }
+} {a b c d}
+test dict-5.9 {dict remove command: canonicality is forced} {
+ dict remove {a b c d a e}
+} {a e c d}
test dict-5.10 {dict remove command: canonicality forced by update} {
dict remove { a b c d } c
} {a b}
@@ -235,6 +239,10 @@ test dict-5.11 {dict remove command: type check is mandatory} -body {
test dict-5.12 {dict remove command: type check is mandatory} -body {
dict remove { a b {}c d }
} -returnCodes error -result {dict element in braces followed by "c" instead of space}
+test dict-5.13 {dict remove command: canonicality forcing doesn't leak} {
+ set example { a b c d }
+ list $example [dict remove $example]
+} {{ a b c d } {a b c d}}
test dict-6.1 {dict keys command} {dict keys {a b}} a
test dict-6.2 {dict keys command} {dict keys {c d}} c