Complete rewrite of DoRead().

1 files changed, 132 insertions, 156 deletions

diff --git a/generic/tclIO.c b/generic/tclIO.c
index 0f894e4..2d22942 100644
--- a/generic/tclIO.c
+++ b/generic/tclIO.c
@@ -11,6 +11,7 @@
  * this file, and for a DISCLAIMER OF ALL WARRANTIES.
  */
 
+#undef NDEBUG
 #include "tclInt.h"
 #include "tclIO.h"
 #include <assert.h>
@@ -173,8 +174,6 @@ static void		CleanupChannelHandlers(Tcl_Interp *interp,
 static int		CloseChannel(Tcl_Interp *interp, Channel *chanPtr,
 			    int errorCode);
 static void		CommonGetsCleanup(Channel *chanPtr);
-static int		CopyAndTranslateBuffer(ChannelState *statePtr,
-			    char *result, int space);
 static int		CopyBuffer(Channel *chanPtr, char *result, int space);
 static int		CopyData(CopyState *csPtr, int mask);
 static void		CopyEventProc(ClientData clientData, int mask);
@@ -188,7 +187,7 @@ static int		DetachChannel(Tcl_Interp *interp, Tcl_Channel chan);
 static void		DiscardInputQueued(ChannelState *statePtr,
 			    int discardSavedBuffers);
 static void		DiscardOutputQueued(ChannelState *chanPtr);
-static int		DoRead(Channel *chanPtr, char *srcPtr, int slen);
+static int		DoRead(Channel *chanPtr, char *dst, int bytesToRead);
 static int		DoReadChars(Channel *chan, Tcl_Obj *objPtr, int toRead,
 			    int appendFlag);
 static int		FilterInputBytes(Channel *chanPtr,
@@ -5363,7 +5362,7 @@ ReadChars(
 	     *	  record \r or \n yet.
 	     */
 
-	    assert(dstRead + 1 == dstDecoded);
+//	    assert(dstRead + 1 == dstDecoded);
 	    assert(dst[dstRead] == '\r');
 	    assert(statePtr->inputTranslation == TCL_TRANSLATE_CRLF);
 
@@ -5384,7 +5383,7 @@ ReadChars(
 
 	    assert(dstWrote == 0);
 	    assert(dstRead == 0);
-	    assert(dstDecoded == 1);
+//	    assert(dstDecoded == 1);
 
 	    /*
 	     * We decoded only the bare cr, and we cannot read a
@@ -5882,6 +5881,9 @@ DiscardInputQueued(
  *
  *	Reads input data from a device into a channel buffer.
  *
+ *	IMPORTANT!  This routine is only called on a chanPtr argument
+ *	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.
@@ -8633,13 +8635,24 @@ CopyData(
  *
  * DoRead --
  *
- *	Reads a given number of bytes from a channel.
+ *	Stores up to "bytesToRead" bytes in memory pointed to by "dst".
+ *	These bytes come from reading the channel "chanPtr" and 
+ *	performing the configured translations.
  *
  *	No encoding conversions are applied to the bytes being read.
  *
  * Results:
- *	The number of characters read, or -1 on error. Use Tcl_GetErrno() to
- *	retrieve the error code for the error that occurred.
+ *	The number of bytes actually stored (<= bytesToRead),
+ * 	or -1 if there is an error in reading the channel.  Use
+ * 	Tcl_GetErrno() to retrieve the error code for the error
+ *	that occurred.
+ *
+ *	The number of bytes stored can be less than the number
+ * 	requested when
+ *	  - EOF is reached on the channel; or
+ *	  - the channel is non-blocking, and we've read all we can
+ *	    without blocking.
+ *	  - a channel reading error occurs (and we return -1)
  *
  * Side effects:
  *	May cause input to be buffered.
@@ -8650,186 +8663,149 @@ CopyData(
 static int
 DoRead(
     Channel *chanPtr,		/* The channel from which to read. */
-    char *bufPtr,		/* Where to store input read. */
-    int toRead)			/* Maximum number of bytes to read. */
+    char *dst,		/* Where to store input read. */
+    int bytesToRead)		/* Maximum number of bytes to read. */
 {
     ChannelState *statePtr = chanPtr->state;
-				/* State info for channel */
-    int copied;			/* How many characters were copied into the
-				 * result string? */
-    int copiedNow;		/* How many characters were copied from the
-				 * current input buffer? */
-    int result;			/* Of calling GetInput. */
+    char *p = dst;
 
-    /*
-     * If we have not encountered a sticky EOF, clear the EOF bit. Either way
-     * clear the BLOCKED bit. We want to discover these anew during each
-     * operation.
-     */
+    while (bytesToRead) {
+	/*
+	 * Each pass through the loop is intended to process up to 
+	 * one channel buffer.
+	 * 
+	 * First, if there is no full buffer, we attempt to 
+	 * create and/or fill one.
+	 */
 
-    if (!(statePtr->flags & CHANNEL_STICKY_EOF)) {
-	ResetFlag(statePtr, CHANNEL_EOF);
-    }
-    ResetFlag(statePtr, CHANNEL_BLOCKED | CHANNEL_NEED_MORE_DATA);
+	ChannelBuffer *bufPtr = statePtr->inQueueHead;
 
-    for (copied = 0; copied < toRead; copied += copiedNow) {
-	copiedNow = CopyAndTranslateBuffer(statePtr, bufPtr + copied,
-		toRead - copied);
-	if (copiedNow == 0) {
-	    if (statePtr->flags & CHANNEL_EOF) {
-		goto done;
-	    }
-	    if (statePtr->flags & CHANNEL_BLOCKED) {
-		if (statePtr->flags & CHANNEL_NONBLOCKING) {
-		    goto done;
-		}
-		ResetFlag(statePtr, CHANNEL_BLOCKED);
+	if (statePtr->flags & CHANNEL_EOF
+		&& (bufPtr == NULL || IsBufferEmpty(bufPtr))) {
+	    break;
+	}
+
+	while (bufPtr == NULL || !IsBufferFull(bufPtr)) {
+	    int code;
+
+	    ResetFlag(statePtr, CHANNEL_BLOCKED);
+	moreData:
+	    code = GetInput(chanPtr);
+	    bufPtr = statePtr->inQueueHead;
+	    if (statePtr->flags & (CHANNEL_EOF|CHANNEL_BLOCKED)) {
+		/* Further reads cannot do any more */
+		break;
 	    }
-	    result = GetInput(chanPtr);
-	    if (result != 0) {
-		if (result != EAGAIN) {
-		    copied = -1;
-		}
-		goto done;
+	    
+	    if (code) {
+		/* Read error */
+		UpdateInterest(chanPtr);
+		return -1;
 	    }
 	}
-    }
 
-    ResetFlag(statePtr, CHANNEL_BLOCKED);
+	/* Here we know bufPtr != NULL */
+	int bytesRead = BytesLeft(bufPtr);
+	int bytesWritten = bytesToRead;
 
-    /*
-     * Update the notifier state so we don't block while there is still data
-     * in the buffers.
-     */
+	if (bytesRead == 0 && statePtr->flags & CHANNEL_NONBLOCKING
+		&& statePtr->flags & CHANNEL_BLOCKED) {
+	    break;
+	}
 
-  done:
-    UpdateInterest(chanPtr);
-    return copied;
-}
-
-/*
- *----------------------------------------------------------------------
- *
- * CopyAndTranslateBuffer --
- *
- *	Copy at most one buffer of input to the result space, doing eol
- *	translations according to mode in effect currently.
- *
- * Results:
- *	Number of bytes stored in the result buffer (as opposed to the number
- *	of bytes read from the channel). May return zero if no input is
- *	available to be translated.
- *
- * Side effects:
- *	Consumes buffered input. May deallocate one buffer.
- *
- *----------------------------------------------------------------------
- */
+	TranslateInputEOL(statePtr, p, RemovePoint(bufPtr),
+		&bytesWritten, &bytesRead);
+	bufPtr->nextRemoved += bytesRead;
+	p += bytesWritten;
+	bytesToRead -= bytesWritten;
 
-static int
-CopyAndTranslateBuffer(
-    ChannelState *statePtr,	/* Channel state from which to read input. */
-    char *result,		/* Where to store the copied input. */
-    int space)			/* How many bytes are available in result to
-				 * store the copied input? */
-{
-    ChannelBuffer *bufPtr;	/* The buffer from which to copy bytes. */
-    int bytesInBuffer;		/* How many bytes are available to be copied
-				 * in the current input buffer? */
-    int copied;			/* How many characters were already copied
-				 * into the destination space? */
+	if (!IsBufferEmpty(bufPtr)) {
+	    /*
+	     * Buffer is not empty.  How can that be?
+	     *
+	     * 0) We stopped early because we got all the bytes
+	     *    we were seeking.  That's fine.
+	     */
 
-    /*
-     * If there is no input at all, return zero. The invariant is that either
-     * there is no buffer in the queue, or if the first buffer is empty, it is
-     * also the last buffer (and thus there is no input in the queue). Note
-     * also that if the buffer is empty, we leave it in the queue.
-     */
+	    if (bytesToRead == 0) {
+		UpdateInterest(chanPtr);
+		break;
+	    }
 
-    if (statePtr->inQueueHead == NULL) {
-	return 0;
-    }
-    bufPtr = statePtr->inQueueHead;
-    bytesInBuffer = BytesLeft(bufPtr);
-    if (bytesInBuffer == 0) {
-	return 0;
-    }
+	    /*
+	     * 1) We're @EOF because we saw eof char.
+	     */
 
-    copied = space;
-    TranslateInputEOL(statePtr, result, RemovePoint(bufPtr),
-	    &copied, &bytesInBuffer);
-    bufPtr->nextRemoved += bytesInBuffer;
+	    if (statePtr->inEofChar
+		    && RemovePoint(bufPtr)[0] == statePtr->inEofChar) {
+		UpdateInterest(chanPtr);
+		break;
+	    }
 
-    /*
-     * If the current buffer is empty recycle it.
-     */
+	    /*
+	     * 2) The buffer holds a \r while in CRLF translation, followed
+	     *    by either the end of the buffer, or the eof char.
+	     */
 
-    if (IsBufferEmpty(bufPtr)) {
-	statePtr->inQueueHead = bufPtr->nextPtr;
-	if (statePtr->inQueueHead == NULL) {
-	    statePtr->inQueueTail = NULL;
-	}
-	RecycleBuffer(statePtr, bufPtr, 0);
-    } else {
+	    assert(statePtr->inputTranslation == TCL_TRANSLATE_CRLF);
+	    assert(RemovePoint(bufPtr)[0] == '\r');
 
-	if (copied > 0) {
-	    return copied;
-	}
+	    if (BytesLeft(bufPtr) > 1) {
 
-	if (statePtr->inEofChar
-		&& RemovePoint(bufPtr)[0] == statePtr->inEofChar) {
-	    return 0;
-	}
+		/* TODO: shift this to TIEOL */
+		assert(statePtr->inEofChar);
+		assert(RemovePoint(bufPtr)[1] == statePtr->inEofChar);
 
-	if (BytesLeft(bufPtr) == 1) {
+		bufPtr->nextRemoved++;
+		*p++ = '\r';
+		bytesToRead--;
+		UpdateInterest(chanPtr);
+		break;
+	    }
 
-	    ChannelBuffer *nextPtr = bufPtr->nextPtr;
+	    assert(BytesLeft(bufPtr) == 1);
 
-	    if (nextPtr == NULL) {
+	    if (bufPtr->nextPtr == NULL) {
+		/* There's no more buffered data.... */
 
 		if (statePtr->flags & CHANNEL_EOF) {
-		    *result = '\r';
-		    bufPtr->nextRemoved += 1;
-		    return 1;
-		}
+		    /* ...and there never will be. */
 
-		SetFlag(statePtr, CHANNEL_NEED_MORE_DATA);
-		return 0;
+		    *p++ = '\r';
+		    bytesToRead--;
+		    bufPtr->nextRemoved++;
+		} else if (statePtr->flags & CHANNEL_BLOCKED) {
+		    /* ...and we cannot get more now. */
+		    SetFlag(statePtr, CHANNEL_NEED_MORE_DATA);
+		    UpdateInterest(chanPtr);
+		    break;
+		} else {
+		    /* ... so we need to get some. */
+		    goto moreData;
+		}
 	    }
 
-	    nextPtr->nextRemoved -= 1;
-	    memcpy(RemovePoint(nextPtr), RemovePoint(bufPtr), 1);
-	    RecycleBuffer(statePtr, bufPtr, 0);
-	    statePtr->inQueueHead = nextPtr;
-	    return 0;
-	}
+	    if (bufPtr->nextPtr) {
+		/* There's a next buffer.  Shift orphan \r to it. */
 
-	if (statePtr->inEofChar
-		&& RemovePoint(bufPtr)[1] == statePtr->inEofChar) {
-	    *result = '\r';
-	    bufPtr->nextRemoved += 1;
-	    return 1;
+		ChannelBuffer *nextPtr = bufPtr->nextPtr;
+
+		nextPtr->nextRemoved -= 1;
+		RemovePoint(nextPtr)[0] = '\r';
+		bufPtr->nextRemoved++;
+	    }
 	}
-	/*
-	 * Buffer is not empty.  How can that be?
-	 * 0) We stopped early due to the value of "space".
-	 * 	=> copied > 0 and all is fine.
-	 * 1) We saw eof char and stopped the translation copy.
-	 *	=> if (copied > 0) or ((copied == 0) and @ eof char),
-	 *		return is fine.
-	 * 2) The buffer holds a \r while in CRLF translation, followed
-	 *    by either the end of the buffer, or the eof char.
-	 */
 
+	if (IsBufferEmpty(bufPtr)) {
+	    statePtr->inQueueHead = bufPtr->nextPtr;
+	    if (statePtr->inQueueHead == NULL) {
+		statePtr->inQueueTail = NULL;
+	    }
+	    RecycleBuffer(statePtr, bufPtr, 0);
+	}
     }
 
-    /*
-     * Return the number of characters copied into the result buffer. This may
-     * be different from the number of bytes consumed, because of EOL
-     * translations.
-     */
-
-    return copied;
+    return (int)(p - dst);
 }
 
 /*