From 32fea5e38c09de7b9f9f19c93074ccdb8a6520d7 Mon Sep 17 00:00:00 2001
From: dgp <dgp@users.sourceforge.net>
Date: Thu, 20 Mar 2014 23:21:48 +0000
Subject: Both callers of ChanRead() have simlar epilogs.  Shift that into
 ChanRead and refactor.

---
 generic/tclIO.c | 123 +++++++++++++++++++++++---------------------------------
 1 file changed, 50 insertions(+), 73 deletions(-)

diff --git a/generic/tclIO.c b/generic/tclIO.c
index e7653f6..18dab5a 100644
--- a/generic/tclIO.c
+++ b/generic/tclIO.c
@@ -348,15 +348,39 @@ static inline int
 ChanRead(
     Channel *chanPtr,
     char *dst,
-    int dstSize,
-    int *errnoPtr)
+    int dstSize)
 {
+    int bytesRead, result;
+
     if (WillRead(chanPtr) < 0) {
         return -1;
     }
 
-    return chanPtr->typePtr->inputProc(chanPtr->instanceData, dst, dstSize,
-	    errnoPtr);
+    bytesRead = chanPtr->typePtr->inputProc(chanPtr->instanceData,
+	    dst, dstSize, &result);
+
+    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 { /* bytesRead < 0 */
+	if ((result == EWOULDBLOCK) || (result == EAGAIN)) {
+	    SetFlag(chanPtr->state, CHANNEL_BLOCKED);
+	    result = EAGAIN;
+	}
+	Tcl_SetErrno(result);
+    }
+    return bytesRead;
 }
 
 static inline Tcl_WideInt
@@ -4833,7 +4857,7 @@ Tcl_ReadRaw(
     Channel *chanPtr = (Channel *) chan;
     ChannelState *statePtr = chanPtr->state;
 				/* State info for channel */
-    int nread, result, copied, copiedNow;
+    int nread, copied, copiedNow;
 
     /*
      * The check below does too much because it will reject a call to this
@@ -4862,11 +4886,11 @@ Tcl_ReadRaw(
 		bytesToRead - copied);
 	if (copiedNow == 0) {
 	    if (statePtr->flags & CHANNEL_EOF) {
-		goto done;
+		break;
 	    }
 	    if (statePtr->flags & CHANNEL_BLOCKED) {
 		if (statePtr->flags & CHANNEL_NONBLOCKING) {
-		    goto done;
+		    break;
 		}
 		ResetFlag(statePtr, CHANNEL_BLOCKED);
 	    }
@@ -4881,48 +4905,21 @@ Tcl_ReadRaw(
 	     * 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.
-			 */
-
-			return copied;
-		    }
+	    nread = ChanRead(chanPtr, bufPtr+copied, bytesToRead-copied);
 
-		    SetFlag(statePtr, CHANNEL_BLOCKED);
-		    result = EAGAIN;
+	    if (nread < 0) {
+		if (statePtr->flags & CHANNEL_BLOCKED && copied > 0) {
+		    ResetFlag(statePtr, CHANNEL_BLOCKED);
+		    break;
 		}
-
-		Tcl_SetErrno(result);
 		return -1;
 	    }
 
-	    return copied + nread;
+	    copied += nread;
+	    break;
 	}
     }
 
-  done:
     return copied;
 }
 
@@ -5019,7 +5016,7 @@ DoReadChars(
     ChannelState *statePtr = chanPtr->state;
 				/* State info for channel */
     ChannelBuffer *bufPtr;
-    int factor, copied, copiedNow, result;
+    int factor, copied, copiedNow;
     Tcl_Encoding encoding;
     int binaryMode;
 #define UTF_EXPANSION_FACTOR	1024
@@ -5090,9 +5087,8 @@ DoReadChars(
 		}
 		ResetFlag(statePtr, CHANNEL_BLOCKED);
 	    }
-	    result = GetInput(chanPtr);
-	    if (result != 0) {
-		if (result == EAGAIN) {
+	    if (GetInput(chanPtr) != 0) {
+		if (statePtr->flags & CHANNEL_BLOCKED) {
 		    break;
 		}
 		copied = -1;
@@ -5883,8 +5879,9 @@ DiscardInputQueued(
  *	that is the top channel of a stack!
  *
  * Results:
- *	The return value is the Posix error code if an error occurred while
- *	reading from the file, or 0 otherwise.
+ *	The return value is 0 to indicate success, or -1 if an error
+ * 	occurred while reading from the channel.  Call Tcl_GetErrno()
+ *	to get the Posix error code.
  *
  * Side effects:
  *	Reads from the underlying device.
@@ -5897,7 +5894,6 @@ GetInput(
     Channel *chanPtr)		/* Channel to read input from. */
 {
     int toRead;			/* How much to read? */
-    int result;			/* Of calling driver. */
     int nread;			/* How much was read from channel? */
     ChannelBuffer *bufPtr;	/* New buffer to add to input queue. */
     ChannelState *statePtr = chanPtr->state;
@@ -5911,7 +5907,8 @@ GetInput(
      */
 
     if (CheckForDeadChannel(NULL, statePtr)) {
-	return EINVAL;
+	Tcl_SetErrno(EINVAL);
+	return -1;
     }
 
     /*
@@ -5988,31 +5985,11 @@ GetInput(
 	return 0;
     }
 
-    nread = ChanRead(chanPtr, InsertPoint(bufPtr), toRead, &result);
-    if (nread > 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.
-	 */
-
-	if (nread < toRead) {
-	    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)) {
-	    SetFlag(statePtr, CHANNEL_BLOCKED);
-	    result = EAGAIN;
-	}
-	Tcl_SetErrno(result);
-	return result;
+    nread = ChanRead(chanPtr, InsertPoint(bufPtr), toRead);
+    if (nread < 0) {
+	return -1;
     }
+    bufPtr->nextAdded += nread;
     return 0;
 }
 
-- 
cgit v0.12


From f27a277c1e1c06a9ddd1c93606a9d884c8d844d7 Mon Sep 17 00:00:00 2001
From: dgp <dgp@users.sourceforge.net>
Date: Fri, 21 Mar 2014 02:36:47 +0000
Subject: Documentation header for ChanRead()

---
 generic/tclIO.c | 25 +++++++++++++++++++++++--
 1 file changed, 23 insertions(+), 2 deletions(-)

diff --git a/generic/tclIO.c b/generic/tclIO.c
index 18dab5a..6e5dc05 100644
--- a/generic/tclIO.c
+++ b/generic/tclIO.c
@@ -342,9 +342,30 @@ 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 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, or
+ *	  - -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,
-- 
cgit v0.12


From e21cda73652e2cf4a060a0411e779935b427a154 Mon Sep 17 00:00:00 2001
From: dgp <dgp@users.sourceforge.net>
Date: Fri, 21 Mar 2014 12:56:46 +0000
Subject: io-34.21 - fix bugs in normally skipped test. io-35.18b - knownBug is
 not buggy on this branch.

---
 tests/io.test | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tests/io.test b/tests/io.test
index cedb337..7f1a357 100644
--- a/tests/io.test
+++ b/tests/io.test
@@ -41,7 +41,7 @@ testConstraint testthread       [llength [info commands testthread]]
 
 # You need a *very* special environment to do some tests.  In
 # particular, many file systems do not support large-files...
-testConstraint largefileSupport 0
+testConstraint largefileSupport 1
 
 # some tests can only be run is umask is 2
 # if "umask" cannot be run, the tests will be skipped.
@@ -4433,10 +4433,10 @@ test io-34.21 {Tcl_Seek and Tcl_Tell on large files} {largefileSupport} {
     puts -nonewline $f abcdef
     lappend l [tell $f]
     close $f
-    lappend l [file size $f]
+    lappend l [file size $path(test3)]
     # truncate...
     close [open $path(test3) w]
-    lappend l [file size $f]
+    lappend l [file size $path(test3)]
     set l
 } {0 6 6 4294967296 4294967302 4294967302 0}
 
@@ -4729,7 +4729,7 @@ test io-35.18a {Tcl_Eof, eof char, cr write, crlf read} -body {
     close $f
     list $s $l $e [scan [string index $in end] %c]
 } -result {9 8 1 13}
-test io-35.18b {Tcl_Eof, eof char, cr write, crlf read} -constraints knownBug -body {
+test io-35.18b {Tcl_Eof, eof char, cr write, crlf read} -body {
     file delete $path(test1)
     set f [open $path(test1) w]
     fconfigure $f -translation cr -eofchar \x1a
-- 
cgit v0.12


From 657b902a0e5afa5596833584329b653f7c0f277d Mon Sep 17 00:00:00 2001
From: dgp <dgp@users.sourceforge.net>
Date: Fri, 21 Mar 2014 12:57:58 +0000
Subject: Fixup ChanRead() header.  Note (dstSize > 0) precondition.

---
 generic/tclIO.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/generic/tclIO.c b/generic/tclIO.c
index 6e5dc05..e7eaefa 100644
--- a/generic/tclIO.c
+++ b/generic/tclIO.c
@@ -346,8 +346,8 @@ static Tcl_ObjType chanObjType = {
  *
  * ChanRead --
  *
- *	Read up to bytes using the inputProc of chanPtr, store them at dst,
- *	and return the number of bytes stored.
+ *	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, 
@@ -373,6 +373,12 @@ ChanRead(
 {
     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);
+
     if (WillRead(chanPtr) < 0) {
         return -1;
     }
-- 
cgit v0.12


From 5bd5d7b876314f563f1ac021424b7f79f45196dd Mon Sep 17 00:00:00 2001
From: dgp <dgp@users.sourceforge.net>
Date: Fri, 21 Mar 2014 12:58:29 +0000
Subject: Restore default suppression of large file test.

---
 tests/io.test | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/io.test b/tests/io.test
index 7f1a357..bfe6051 100644
--- a/tests/io.test
+++ b/tests/io.test
@@ -41,7 +41,7 @@ testConstraint testthread       [llength [info commands testthread]]
 
 # You need a *very* special environment to do some tests.  In
 # particular, many file systems do not support large-files...
-testConstraint largefileSupport 1
+testConstraint largefileSupport 0
 
 # some tests can only be run is umask is 2
 # if "umask" cannot be run, the tests will be skipped.
-- 
cgit v0.12


From 6b106c4d93bfc4a452a3cf9d78aa3ee25761e553 Mon Sep 17 00:00:00 2001
From: dgp <dgp@users.sourceforge.net>
Date: Fri, 21 Mar 2014 13:20:06 +0000
Subject: Convert "impossible" test to a "knownBug" test.  Exposes a segfault!

---
 tests/ioCmd.test | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/tests/ioCmd.test b/tests/ioCmd.test
index 768a748..8f0bfbf 100644
--- a/tests/ioCmd.test
+++ b/tests/ioCmd.test
@@ -1927,6 +1927,8 @@ test iocmd-32.0 {origin interpreter of moved channel gone} -match glob -body {
 
 test iocmd-32.1 {origin interpreter of moved channel destroyed during access} -match glob -body {
 
+    # This test segfaults;  Ought to fix that.
+
     set ida [interp create];#puts <<$ida>>
     set idb [interp create];#puts <<$idb>>
 
@@ -1940,13 +1942,12 @@ test iocmd-32.1 {origin interpreter of moved channel destroyed during access} -m
 	proc foo {args} {
 	    oninit; onfinal; track;
 	    # destroy interpreter during channel access
-	    # Actually not possible for an interp to destroy itself.
-	    interp delete {}
-	    return}
+	    suicide}
 	set chan [chan create {r w} foo]
 	fconfigure $chan -buffering none
 	set chan
     }]
+    interp alias $ida suicide {} interp delete $ida
 
     # Move channel to 2nd thread.
     interp eval $ida [list testchannel cut    $chan]
@@ -1965,7 +1966,7 @@ test iocmd-32.1 {origin interpreter of moved channel destroyed during access} -m
 	set res
     }]
     set res
-} -constraints {testchannel impossible} \
+} -constraints {testchannel knownBug} \
     -result {Owner lost}
 
 test iocmd-32.2 {delete interp of reflected chan} {
-- 
cgit v0.12


From f715783b1bff0fe44a894ff26049debf8cb184f2 Mon Sep 17 00:00:00 2001
From: dgp <dgp@users.sourceforge.net>
Date: Fri, 21 Mar 2014 13:40:58 +0000
Subject: Added comment explaining the "knownBug"

---
 tests/iogt.test | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/tests/iogt.test b/tests/iogt.test
index 3882ecc..3b94747 100644
--- a/tests/iogt.test
+++ b/tests/iogt.test
@@ -916,6 +916,15 @@ test iogt-6.0 {Push back} testchannel {
 } {xxx}
 
 test iogt-6.1 {Push back and up} {testchannel knownBug} {
+
+    # This test demonstrates the bug/misfeature in the stacked
+    # channel implementation that data can be discarded if it is
+    # read into the buffers of one channel in the stack, and then
+    # that channel is popped before anything above it reads.
+    #
+    # This bug can be worked around by always setting -buffersize
+    # to 1, but who wants to do that?
+
     set f [open $path(dummy) r]
 
     # contents of dummy = "abcdefghi..."
-- 
cgit v0.12


From 129e8e28a57eb14bc1a2c5e06dd60f90124ab2bf Mon Sep 17 00:00:00 2001
From: dgp <dgp@users.sourceforge.net>
Date: Fri, 21 Mar 2014 14:07:40 +0000
Subject: Correct namespace bugs in normally skipped tests. Constrain them as
 "knownBug" rather than "unknownFailure".

---
 tests/iogt.test | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/tests/iogt.test b/tests/iogt.test
index 3b94747..0a25418 100644
--- a/tests/iogt.test
+++ b/tests/iogt.test
@@ -159,8 +159,8 @@ proc fevent {fdelay idelay blocks script data} {
 
     #puts stdout ">>>>>" ; flush stdout
 
-    uplevel #0 set sock $sk
-    set res [uplevel #0 $script]
+    uplevel 1 set sock $sk
+    set res [uplevel 1 $script]
 
     catch {close $sk}
     return $res
@@ -634,7 +634,7 @@ delete/write {} *ignored*}
 
 
 test iogt-3.0 {Tcl_Channel valid after stack/unstack, fevent handling} \
-	{testchannel unknownFailure} {
+	{testchannel knownBug} {
     # This test to check the validity of aquired Tcl_Channel references is
     # not possible because even a backgrounded fcopy will immediately start
     # to copy data, without waiting for the event loop. This is done only in
@@ -651,6 +651,7 @@ test iogt-3.0 {Tcl_Channel valid after stack/unstack, fevent handling} \
     set fin  [open $path(dummy) r]
 
     fevent 1000 500 {20 20 20 10 1 1} {
+	variable copy
 	close $fin
 
 	set          fout [open dummyout w]
@@ -688,7 +689,7 @@ test iogt-3.0 {Tcl_Channel valid after stack/unstack, fevent handling} \
 } {1 {create/write create/read write flush/write flush/read delete/write delete/read}}
 
 
-test iogt-4.0 {fileevent readable, after transform} {testchannel unknownFailure} {
+test iogt-4.0 {fileevent readable, after transform} {testchannel knownBug} {
     set fin  [open $path(dummy) r]
     set data [read $fin]
     close $fin
@@ -718,10 +719,11 @@ test iogt-4.0 {fileevent readable, after transform} {testchannel unknownFailure}
     }
 
     fevent 1000 500 {20 20 20 10 1} {
+	variable stop
 	audit_flow trail   -attach $sock
 	rblocks_t  rbuf trail 23 -attach $sock
 
-	fileevent $sock readable [list Get $sock]
+	fileevent $sock readable [namespace code [list Get $sock]]
 
 	flush $sock ; # now, or fcopy will error us out
 	# But the 1 second delay should be enough to
@@ -819,7 +821,7 @@ delete/write {} *ignored*
 delete/read {} *ignored*}  ; # catch unescaped quote "
 
 
-test iogt-5.0 {EOF simulation} {testchannel unknownFailure} {
+test iogt-5.0 {EOF simulation} {testchannel knownBug} {
     set fin  [open $path(dummy)    r]
     set fout [open $path(dummyout) w]
 
-- 
cgit v0.12


From 2266861ddbdc18e12918eb94efeefc37edc894f8 Mon Sep 17 00:00:00 2001
From: dgp <dgp@users.sourceforge.net>
Date: Fri, 21 Mar 2014 14:45:00 +0000
Subject: missing declaration

---
 generic/tclIO.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/generic/tclIO.c b/generic/tclIO.c
index e7eaefa..a62f80e 100644
--- a/generic/tclIO.c
+++ b/generic/tclIO.c
@@ -165,6 +165,7 @@ typedef struct CloseCallback {
 
 static ChannelBuffer *	AllocChannelBuffer(int length);
 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,
-- 
cgit v0.12


From c3df58587ce6e9f21b652b23eb7f56f852a326f7 Mon Sep 17 00:00:00 2001
From: dgp <dgp@users.sourceforge.net>
Date: Tue, 13 May 2014 18:24:23 +0000
Subject: Rework Tcl_ReadRaw() mostly taking things out of the loop that never
 repeat.

---
 generic/tclIO.c | 56 ++++++++++++++++++++------------------------------------
 1 file changed, 20 insertions(+), 36 deletions(-)

diff --git a/generic/tclIO.c b/generic/tclIO.c
index 41f555b..a82c36b 100644
--- a/generic/tclIO.c
+++ b/generic/tclIO.c
@@ -4985,7 +4985,7 @@ Tcl_ReadRaw(
     Channel *chanPtr = (Channel *) chan;
     ChannelState *statePtr = chanPtr->state;
 				/* State info for channel */
-    int nread, copied, copiedNow;
+    int nread, copied, copiedNow = INT_MAX;
 
     /*
      * The check below does too much because it will reject a call to this
@@ -5010,44 +5010,28 @@ 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)) {
-		break;
-	    }
-	    if (GotFlag(statePtr, CHANNEL_BLOCKED)) {
-		if (GotFlag(statePtr, CHANNEL_NONBLOCKING)) {
-		    break;
-		}
-		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.
-	     */
+    for (copied = 0; bytesToRead > 0 && copiedNow > 0;
+	    bufPtr+=copiedNow, bytesToRead-=copiedNow, copied+=copiedNow) {
+	copiedNow = CopyBuffer(chanPtr, bufPtr, bytesToRead);
+    }
 
-	    nread = ChanRead(chanPtr, bufPtr + copied,
-		    bytesToRead - copied);
+    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.
+	 */
 
-	    if (nread < 0) {
-		if (GotFlag(statePtr, CHANNEL_BLOCKED) && copied > 0) {
-/* TODO: comment out? */
-//		    ResetFlag(statePtr, CHANNEL_BLOCKED);
-		} else {
-		    copied = -1;
-		}
-	    } else {
-		copied += nread;
+	nread = ChanRead(chanPtr, bufPtr, bytesToRead);
+	if (nread < 0) {
+	    if (!GotFlag(statePtr, CHANNEL_BLOCKED) || copied == 0) {
+		copied = -1;
 	    }
-	    break;
+	} else {
+	    copied += nread;
+	}
+	if (copied != 0) {
+	    ResetFlag(statePtr, CHANNEL_EOF);
 	}
     }
 
-- 
cgit v0.12

