summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authordgp <dgp@users.sourceforge.net>2016-07-18 16:26:44 (GMT)
committerdgp <dgp@users.sourceforge.net>2016-07-18 16:26:44 (GMT)
commita127a9e4aa8b2a9a386cd7fe694bfef8f1cf5264 (patch)
tree6e8a2ea347e991687484927a565d7b911e59d7e5
parent8f0cfce66a4fd94536faac0595c3d7d7c7844d64 (diff)
downloadtcl-a127a9e4aa8b2a9a386cd7fe694bfef8f1cf5264.zip
tcl-a127a9e4aa8b2a9a386cd7fe694bfef8f1cf5264.tar.gz
tcl-a127a9e4aa8b2a9a386cd7fe694bfef8f1cf5264.tar.bz2
[104f2885bb] Rework the "chan" Tcl_ObjType to properly validate cached
channel name lookups.
-rw-r--r--generic/tclIO.c161
-rw-r--r--generic/tclIO.h6
-rw-r--r--tests/io.test19
3 files changed, 97 insertions, 89 deletions
diff --git a/generic/tclIO.c b/generic/tclIO.c
index 7bc849e..615fe60 100644
--- a/generic/tclIO.c
+++ b/generic/tclIO.c
@@ -312,14 +312,20 @@ static int WillRead(Channel *chanPtr);
&& (strncmp(optionName, (nameString), len) == 0))
/*
- * The ChannelObjType type. We actually store the ChannelState structure
- * as that lives longest and we want to return the bottomChanPtr when
- * requested (consistent with Tcl_GetChannel). The setFromAny and
- * updateString can be NULL as they should not be called.
+ * The ChannelObjType type. Used to store the result of looking up
+ * a channel name in the context of an interp. Saves the lookup
+ * result and values needed to check its continued validity.
*/
+typedef struct ResolvedChanName {
+ ChannelState *statePtr; /* The saved lookup result */
+ Tcl_Interp *interp; /* The interp in which the lookup was done. */
+ int epoch; /* The epoch of the channel when the lookup
+ * was done. Use to verify validity. */
+ int refCount; /* Share this struct among many Tcl_Obj. */
+} ResolvedChanName;
+
static void DupChannelIntRep(Tcl_Obj *objPtr, Tcl_Obj *copyPtr);
-static int SetChannelFromAny(Tcl_Interp *interp, Tcl_Obj *objPtr);
static void FreeChannelIntRep(Tcl_Obj *objPtr);
static Tcl_ObjType chanObjType = {
@@ -327,18 +333,9 @@ static Tcl_ObjType chanObjType = {
FreeChannelIntRep, /* freeIntRepProc */
DupChannelIntRep, /* dupIntRepProc */
NULL, /* updateStringProc */
- NULL /* setFromAnyProc SetChannelFromAny */
+ NULL /* setFromAnyProc */
};
-#define GET_CHANNELSTATE(objPtr) \
- ((ChannelState *) (objPtr)->internalRep.twoPtrValue.ptr1)
-#define SET_CHANNELSTATE(objPtr, storePtr) \
- ((objPtr)->internalRep.twoPtrValue.ptr1 = (void *) (storePtr))
-#define GET_CHANNELINTERP(objPtr) \
- ((Tcl_Interp *) (objPtr)->internalRep.twoPtrValue.ptr2)
-#define SET_CHANNELINTERP(objPtr, storePtr) \
- ((objPtr)->internalRep.twoPtrValue.ptr2 = (void *) (storePtr))
-
#define BUSY_STATE(st,fl) \
((((st)->csPtrR) && ((fl) & TCL_READABLE)) || \
(((st)->csPtrW) && ((fl) & TCL_WRITABLE)))
@@ -936,7 +933,7 @@ DeleteChannelTable(
*/
Tcl_DeleteHashEntry(hPtr);
- SetFlag(statePtr, CHANNEL_TAINTED);
+ statePtr->epoch++;
statePtr->refCount--;
if (statePtr->refCount <= 0) {
if (!GotFlag(statePtr, BG_FLUSH_SCHEDULED)) {
@@ -1280,7 +1277,7 @@ DetachChannel(
return TCL_ERROR;
}
Tcl_DeleteHashEntry(hPtr);
- SetFlag(statePtr, CHANNEL_TAINTED);
+ statePtr->epoch++;
/*
* Remove channel handlers that refer to this interpreter, so that
@@ -1413,12 +1410,57 @@ TclGetChannelFromObj(
int flags)
{
ChannelState *statePtr;
+ ResolvedChanName *resPtr = NULL;
+ Tcl_Channel chan;
+
+ if (interp == NULL) {
+ return TCL_ERROR;
+ }
+
+ if (objPtr->typePtr == &chanObjType) {
+ /*
+ * Confirm validity of saved lookup results.
+ */
+
+ resPtr = (ResolvedChanName *) objPtr->internalRep.twoPtrValue.ptr1;
+ statePtr = resPtr->statePtr;
+ if ((resPtr->interp == interp) /* Same interp context */
+ /* No epoch change in channel since lookup */
+ && (resPtr->epoch == statePtr->epoch)) {
+
+ /* Have a valid saved lookup. Jump to end to return it. */
+ goto valid;
+ }
+ }
+
+ chan = Tcl_GetChannel(interp, TclGetString(objPtr), NULL);
- if (SetChannelFromAny(interp, objPtr) != TCL_OK) {
+ if (chan == NULL) {
+ if (resPtr) {
+ FreeChannelIntRep(objPtr);
+ }
return TCL_ERROR;
}
- statePtr = GET_CHANNELSTATE(objPtr);
+ if (resPtr && resPtr->refCount == 1) {
+ /* Re-use the ResolvedCmdName struct */
+ Tcl_Release((ClientData) resPtr->statePtr);
+
+ } else {
+ TclFreeIntRep(objPtr);
+
+ resPtr = (ResolvedChanName *) ckalloc(sizeof(ResolvedChanName));
+ resPtr->refCount = 1;
+ objPtr->internalRep.twoPtrValue.ptr1 = (ClientData) resPtr;
+ objPtr->typePtr = &chanObjType;
+ }
+ statePtr = ((Channel *)chan)->state;
+ resPtr->statePtr = statePtr;
+ Tcl_Preserve((ClientData) statePtr);
+ resPtr->interp = interp;
+ resPtr->epoch = statePtr->epoch;
+
+ valid:
*channelPtr = (Tcl_Channel) (statePtr->bottomChanPtr);
if (modePtr != NULL) {
@@ -1568,6 +1610,8 @@ Tcl_CreateChannel(
statePtr->chanMsg = NULL;
statePtr->unreportedMsg = NULL;
+ statePtr->epoch = 0;
+
/*
* Link the channel into the list of all channels; create an on-exit
* handler if there is not one already, to close off all the channels in
@@ -10439,78 +10483,17 @@ DupChannelIntRep(
register Tcl_Obj *copyPtr) /* Object with internal rep to set. Must not
* currently have an internal rep.*/
{
- ChannelState *statePtr = GET_CHANNELSTATE(srcPtr);
+ ResolvedChanName *resPtr
+ = (ResolvedChanName *) srcPtr->internalRep.twoPtrValue.ptr1;
- SET_CHANNELSTATE(copyPtr, statePtr);
- SET_CHANNELINTERP(copyPtr, GET_CHANNELINTERP(srcPtr));
- Tcl_Preserve((ClientData) statePtr);
+ resPtr->refCount++;
+ copyPtr->internalRep.twoPtrValue.ptr1 = resPtr;
copyPtr->typePtr = srcPtr->typePtr;
}
/*
*----------------------------------------------------------------------
*
- * SetChannelFromAny --
- *
- * Create an internal representation of type "Channel" for an object.
- *
- * Results:
- * This operation always succeeds and returns TCL_OK.
- *
- * Side effects:
- * Any old internal reputation for objPtr is freed and the internal
- * representation is set to "Channel".
- *
- *----------------------------------------------------------------------
- */
-
-static int
-SetChannelFromAny(
- Tcl_Interp *interp, /* Used for error reporting if not NULL. */
- register Tcl_Obj *objPtr) /* The object to convert. */
-{
- ChannelState *statePtr;
-
- if (interp == NULL) {
- return TCL_ERROR;
- }
- if (objPtr->typePtr == &chanObjType) {
- /*
- * TODO: TAINT Flag and dup'd channel values?
- * The channel is valid until any call to DetachChannel occurs.
- * Ensure consistency checks are done.
- */
-
- statePtr = GET_CHANNELSTATE(objPtr);
- if (GotFlag(statePtr, CHANNEL_TAINTED|CHANNEL_CLOSED)) {
- ResetFlag(statePtr, CHANNEL_TAINTED);
- Tcl_Release((ClientData) statePtr);
- objPtr->typePtr = NULL;
- } else if (interp != GET_CHANNELINTERP(objPtr)) {
- Tcl_Release((ClientData) statePtr);
- objPtr->typePtr = NULL;
- }
- }
- if (objPtr->typePtr != &chanObjType) {
- Tcl_Channel chan = Tcl_GetChannel(interp, TclGetString(objPtr), NULL);
-
- if (chan == NULL) {
- return TCL_ERROR;
- }
-
- TclFreeIntRep(objPtr);
- statePtr = ((Channel *)chan)->state;
- Tcl_Preserve((ClientData) statePtr);
- SET_CHANNELSTATE(objPtr, statePtr);
- SET_CHANNELINTERP(objPtr, interp);
- objPtr->typePtr = &chanObjType;
- }
- return TCL_OK;
-}
-
-/*
- *----------------------------------------------------------------------
- *
* FreeChannelIntRep --
*
* Release statePtr storage.
@@ -10528,7 +10511,15 @@ static void
FreeChannelIntRep(
Tcl_Obj *objPtr) /* Object with internal rep to free. */
{
- Tcl_Release((ClientData) GET_CHANNELSTATE(objPtr));
+ ResolvedChanName *resPtr
+ = (ResolvedChanName *) objPtr->internalRep.twoPtrValue.ptr1;
+
+ objPtr->typePtr = NULL;
+ if (--resPtr->refCount) {
+ return;
+ }
+ Tcl_Release((ClientData) resPtr->statePtr);
+ ckfree((char *)resPtr);
}
#if 0
diff --git a/generic/tclIO.h b/generic/tclIO.h
index 348a9c5..d096798 100644
--- a/generic/tclIO.h
+++ b/generic/tclIO.h
@@ -214,6 +214,8 @@ typedef struct ChannelState {
* because it happened in the background. The
* value is the chanMg, if any. #219's
* companion to 'unreportedError'. */
+ int epoch; /* Used to test validity of stored channelname
+ * lookup results. */
} ChannelState;
/*
@@ -275,10 +277,6 @@ typedef struct ChannelState {
* usable, but it may not be closed
* again from within the close
* handler. */
-#define CHANNEL_TAINTED (1<<20) /* Channel stack structure has changed.
- * Used by Channel Tcl_Obj type to
- * determine if we have to revalidate
- * the channel. */
/*
* Local Variables:
diff --git a/tests/io.test b/tests/io.test
index 50c5808..7275b42 100644
--- a/tests/io.test
+++ b/tests/io.test
@@ -38,6 +38,7 @@ testConstraint testfevent [llength [info commands testfevent]]
testConstraint testchannelevent [llength [info commands testchannelevent]]
testConstraint testmainthread [llength [info commands testmainthread]]
testConstraint testthread [llength [info commands testthread]]
+testConstraint testobj [llength [info commands testobj]]
# You need a *very* special environment to do some tests. In
# particular, many file systems do not support large-files...
@@ -8522,6 +8523,24 @@ test io-73.5 {effect of eof on encoding end flags} -setup {
removeFile io-73.5
} -result [list 1 1 more\u00a0data 1]
+test io-74.1 {[104f2885bb] improper cache validity check} -setup {
+ set fn [makeFile {} io-74.1]
+ set rfd [open $fn r]
+ testobj freeallvars
+ interp create slave
+} -constraints testobj -body {
+ teststringobj set 1 [string range $rfd 0 end]
+ read [teststringobj get 1]
+ testobj duplicate 1 2
+ interp transfer {} $rfd slave
+ catch {read [teststringobj get 1]}
+ read [teststringobj get 2]
+} -cleanup {
+ interp delete slave
+ testobj freeallvars
+ removeFile io-74.1
+} -returnCodes error -match glob -result {can not find channel named "*"}
+
# ### ### ### ######### ######### #########
# cleanup