diff options
| -rw-r--r-- | generic/tclIO.c | 161 | ||||
| -rw-r--r-- | generic/tclIO.h | 6 | ||||
| -rw-r--r-- | tests/io.test | 19 |
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 |