diff options
| -rw-r--r-- | generic/tclIO.c | 223 | ||||
| -rw-r--r-- | tests/io.test | 16 |
2 files changed, 127 insertions, 112 deletions
diff --git a/generic/tclIO.c b/generic/tclIO.c index 7a53373..a5c77e8 100644 --- a/generic/tclIO.c +++ b/generic/tclIO.c @@ -12,6 +12,7 @@ * this file, and for a DISCLAIMER OF ALL WARRANTIES. */ +#undef NDEBUG #include "tclInt.h" #include "tclIO.h" #include <assert.h> @@ -167,6 +168,7 @@ static void PreserveChannelBuffer(ChannelBuffer *bufPtr); static void ReleaseChannelBuffer(ChannelBuffer *bufPtr); static int IsShared(ChannelBuffer *bufPtr); static void ChannelTimerProc(ClientData clientData); +static int ChanRead(Channel *chanPtr, char *dst, int dstSize); static int CheckChannelErrors(ChannelState *statePtr, int direction); static int CheckForDeadChannel(Tcl_Interp *interp, @@ -345,22 +347,78 @@ static Tcl_ObjType chanObjType = { #define MAX_CHANNEL_BUFFER_SIZE (1024*1024) /* - * ChanRead, dropped here by a time traveler, see 8.6 + *--------------------------------------------------------------------------- + * + * ChanRead -- + * + * Read up to dstSize bytes using the inputProc of chanPtr, store + * them at dst, and return the number of bytes stored. + * + * Results: + * The return value of the driver inputProc, + * - number of bytes stored at dst, ot + * - -1 on error, with a Posix error code available to the + * caller by calling Tcl_GetErrno(). + * + * Side effects: + * The CHANNEL_BLOCKED and CHANNEL_EOF flags of the channel state are + * set as appropriate. + * On EOF, the inputEncodingFlags are set to perform ending operations + * on decoding. + * TODO - Is this really the right place for that? + * + *--------------------------------------------------------------------------- */ -static inline int +static int ChanRead( Channel *chanPtr, char *dst, - int dstSize, - int *errnoPtr) + int dstSize) { + int bytesRead, result; + + /* + * If the caller asked for zero bytes, we'd force the inputProc + * to return zero bytes, and then misinterpret that as EOF. + */ + assert(dstSize > 0); + + /* + * Each read op must set the blocked and eof states anew, not let + * the effect of prior reads leak through. + */ + ResetFlag(chanPtr->state, CHANNEL_BLOCKED | CHANNEL_EOF); if (WillRead(chanPtr) < 0) { - *errnoPtr = Tcl_GetErrno(); return -1; } - return chanPtr->typePtr->inputProc(chanPtr->instanceData, dst, dstSize, - errnoPtr); + bytesRead = chanPtr->typePtr->inputProc(chanPtr->instanceData, + dst, dstSize, &result); + + /* Stop any flag leakage through stacked channel levels */ + ResetFlag(chanPtr->state, CHANNEL_BLOCKED | CHANNEL_EOF); + if (bytesRead > 0) { + /* + * If we get a short read, signal up that we may be BLOCKED. + * We should avoid calling the driver because on some + * platforms we will block in the low level reading code even + * though the channel is set into nonblocking mode. + */ + + if (bytesRead < dstSize) { + SetFlag(chanPtr->state, CHANNEL_BLOCKED); + } + } else if (bytesRead == 0) { + SetFlag(chanPtr->state, CHANNEL_EOF); + chanPtr->state->inputEncodingFlags |= TCL_ENCODING_END; + } else if (bytesRead < 0) { + if ((result == EWOULDBLOCK) || (result == EAGAIN)) { + SetFlag(chanPtr->state, CHANNEL_BLOCKED); + result = EAGAIN; + } + Tcl_SetErrno(result); + } + return bytesRead; } static inline Tcl_WideInt @@ -3891,6 +3949,9 @@ Tcl_GetsObj( goto done; } + /* TODO: Locate better place(s) to reset this flag */ + ResetFlag(statePtr, CHANNEL_BLOCKED); + /* * A binary version of Tcl_GetsObj. This could also handle encodings that * are ascii-7 pure (iso8859, utf-8, ...) with a final encoding conversion @@ -4324,7 +4385,6 @@ TclGetsObjBinary( if (GotFlag(statePtr, CHANNEL_NONBLOCKING)) { goto restore; } - ResetFlag(statePtr, CHANNEL_BLOCKED); } if (GetInput(chanPtr) != 0) { goto restore; @@ -4590,13 +4650,13 @@ FilterInputBytes( */ read: + /* TODO: Move this check to the goto */ if (GotFlag(statePtr, CHANNEL_BLOCKED)) { if (GotFlag(statePtr, CHANNEL_NONBLOCKING)) { gsPtr->charsWrote = 0; gsPtr->rawRead = 0; return -1; } - ResetFlag(statePtr, CHANNEL_BLOCKED); } if (GetInput(chanPtr) != 0) { gsPtr->charsWrote = 0; @@ -4932,7 +4992,7 @@ Tcl_ReadRaw( Channel *chanPtr = (Channel *) chan; ChannelState *statePtr = chanPtr->state; /* State info for channel */ - int nread, result, copied, copiedNow; + int nread, copied, copiedNow = INT_MAX; /* * The check below does too much because it will reject a call to this @@ -4957,74 +5017,31 @@ Tcl_ReadRaw( */ Tcl_Preserve(chanPtr); - for (copied = 0; copied < bytesToRead; copied += copiedNow) { - copiedNow = CopyBuffer(chanPtr, bufPtr + copied, - bytesToRead - copied); - if (copiedNow == 0) { - if (GotFlag(statePtr, CHANNEL_EOF)) { - goto done; - } - if (GotFlag(statePtr, CHANNEL_BLOCKED)) { - if (GotFlag(statePtr, CHANNEL_NONBLOCKING)) { - goto done; - } - ResetFlag(statePtr, CHANNEL_BLOCKED); - } - - /* - * Now go to the driver to get as much as is possible to - * fill the remaining request. Do all the error handling by - * ourselves. The code was stolen from 'GetInput' and - * slightly adapted (different return value here). - * - * The case of 'bytesToRead == 0' at this point cannot - * happen. - */ - - nread = ChanRead(chanPtr, bufPtr + copied, - bytesToRead - copied, &result); - - if (nread > 0) { - /* - * If we get a short read, signal up that we may be BLOCKED. - * We should avoid calling the driver because on some - * platforms we will block in the low level reading code even - * though the channel is set into nonblocking mode. - */ - - if (nread < (bytesToRead - copied)) { - SetFlag(statePtr, CHANNEL_BLOCKED); - } - } else if (nread == 0) { - SetFlag(statePtr, CHANNEL_EOF); - statePtr->inputEncodingFlags |= TCL_ENCODING_END; - - } else if (nread < 0) { - if ((result == EWOULDBLOCK) || (result == EAGAIN)) { - if (copied > 0) { - /* - * Information that was copied earlier has precedence - * over EAGAIN/WOULDBLOCK handling. - */ - - goto done; - } + for (copied = 0; bytesToRead > 0 && copiedNow > 0; + bufPtr+=copiedNow, bytesToRead-=copiedNow, copied+=copiedNow) { + copiedNow = CopyBuffer(chanPtr, bufPtr, bytesToRead); + } - SetFlag(statePtr, CHANNEL_BLOCKED); - result = EAGAIN; - } + if (bytesToRead > 0) { + /* + * Now go to the driver to get as much as is possible to + * fill the remaining request. Since we're directly filling + * the caller's buffer, retain the blocked flag. + */ - Tcl_SetErrno(result); + nread = ChanRead(chanPtr, bufPtr, bytesToRead); + if (nread < 0) { + if (!GotFlag(statePtr, CHANNEL_BLOCKED) || copied == 0) { copied = -1; - goto done; } - + } else { copied += nread; - goto done; + } + if (copied != 0) { + ResetFlag(statePtr, CHANNEL_EOF); } } - done: Tcl_Release(chanPtr); return copied; } @@ -5158,6 +5175,8 @@ DoReadChars( } } + /* Must clear the BLOCKED flag here since we check before reading */ + ResetFlag(statePtr, CHANNEL_BLOCKED); for (copied = 0; (unsigned) toRead > 0; ) { copiedNow = -1; if (statePtr->inQueueHead != NULL) { @@ -5192,7 +5211,6 @@ DoReadChars( if (GotFlag(statePtr, CHANNEL_NONBLOCKING)) { break; } - ResetFlag(statePtr, CHANNEL_BLOCKED); } result = GetInput(chanPtr); if (chanPtr != statePtr->topChanPtr) { @@ -5201,11 +5219,10 @@ DoReadChars( Tcl_Preserve(chanPtr); } if (result != 0) { - if (result == EAGAIN) { - break; + if (!GotFlag(statePtr, CHANNEL_BLOCKED)) { + copied = -1; } - copied = -1; - goto done; + break; } } else { copied += copiedNow; @@ -5213,14 +5230,15 @@ DoReadChars( } } - ResetFlag(statePtr, CHANNEL_BLOCKED); - /* - * Update the notifier state so we don't block while there is still data - * in the buffers. + * Failure to fill a channel buffer may have left channel reporting + * a "blocked" state, but so long as we fulfilled the request here, + * the caller does not consider us blocked. */ + if (toRead == 0) { + ResetFlag(statePtr, CHANNEL_BLOCKED); + } - done: /* * Regenerate the top channel, in case it was changed due to * self-modifying reflected transforms. @@ -5230,6 +5248,11 @@ DoReadChars( chanPtr = statePtr->topChanPtr; Tcl_Preserve(chanPtr); } + + /* + * Update the notifier state so we don't block while there is still data + * in the buffers. + */ UpdateInterest(chanPtr); Tcl_Release(chanPtr); return copied; @@ -6136,32 +6159,15 @@ GetInput( } PreserveChannelBuffer(bufPtr); - nread = ChanRead(chanPtr, InsertPoint(bufPtr), toRead, &result); - if (nread > 0) { - result = 0; - bufPtr->nextAdded += nread; - - /* - * If we get a short read, signal up that we may be BLOCKED. We should - * avoid calling the driver because on some platforms we will block in - * the low level reading code even though the channel is set into - * nonblocking mode. - */ + nread = ChanRead(chanPtr, InsertPoint(bufPtr), toRead); - if (nread < toRead) { - SetFlag(statePtr, CHANNEL_BLOCKED); - } - } else if (nread == 0) { + if (nread < 0) { + result = Tcl_GetErrno(); + } else { result = 0; - SetFlag(statePtr, CHANNEL_EOF); - statePtr->inputEncodingFlags |= TCL_ENCODING_END; - } else if (nread < 0) { - if ((result == EWOULDBLOCK) || (result == EAGAIN)) { - SetFlag(statePtr, CHANNEL_BLOCKED); - result = EAGAIN; - } - Tcl_SetErrno(result); + bufPtr->nextAdded += nread; } + ReleaseChannelBuffer(bufPtr); return result; } @@ -6649,12 +6655,7 @@ CheckChannelErrors( } if (direction == TCL_READABLE) { - /* - * Clear the BLOCKED bit. We want to discover this condition - * anew in each operation. - */ - - ResetFlag(statePtr, CHANNEL_BLOCKED | CHANNEL_NEED_MORE_DATA); + ResetFlag(statePtr, CHANNEL_NEED_MORE_DATA); } return 0; @@ -8831,7 +8832,6 @@ DoRead( while (bufPtr == NULL || !IsBufferFull(bufPtr)) { int code; - ResetFlag(statePtr, CHANNEL_BLOCKED); moreData: code = GetInput(chanPtr); bufPtr = statePtr->inQueueHead; @@ -8940,6 +8940,9 @@ DoRead( break; } } + if (bytesToRead == 0) { + ResetFlag(statePtr, CHANNEL_BLOCKED); + } Tcl_Release(chanPtr); return (int)(p - dst); diff --git a/tests/io.test b/tests/io.test index 5f31d8e..a2e2397 100644 --- a/tests/io.test +++ b/tests/io.test @@ -6705,11 +6705,23 @@ test io-52.4 {TclCopyChannel} {fcopy} { fconfigure $f1 -translation lf -blocking 0 fconfigure $f2 -translation cr -blocking 0 fcopy $f1 $f2 -size 40 - set result [list [fconfigure $f1 -blocking] [fconfigure $f2 -blocking]] + set result [list [fblocked $f1] [fconfigure $f1 -blocking] [fconfigure $f2 -blocking]] + close $f1 + close $f2 + lappend result [file size $path(test1)] +} {0 0 0 40} +test io-52.4.1 {TclCopyChannel} {fcopy} { + file delete $path(test1) + set f1 [open $thisScript] + set f2 [open $path(test1) w] + fconfigure $f1 -translation lf -blocking 0 -buffersize 10000000 + fconfigure $f2 -translation cr -blocking 0 + fcopy $f1 $f2 -size 40 + set result [list [fblocked $f1] [fconfigure $f1 -blocking] [fconfigure $f2 -blocking]] close $f1 close $f2 lappend result [file size $path(test1)] -} {0 0 40} +} {0 0 0 40} test io-52.5 {TclCopyChannel, all} {fcopy} { file delete $path(test1) set f1 [open $thisScript] |