Re: Pty is losing bytes

[Roman Zippel]

Hi,

On Tue, 15 Feb 2005, Linus Torvalds wrote:

> So I'd still worry whether that added -1 actually fixes the bug, or just 
> means that a off-by-one has to now be off-by-two to be noticeable..

You're right, even if one just writes 4095 bytes, it will also drop 
tty->read_cnt characters later.

> Does that make sense to you? Maybe the "input full, but no canon_data" 
> special case would be better handled in the read path, rather than the 
> write path?

There might be no reader at this time. The basic problem is that this 
should be handled in the receive_buf path, but for that it had to return 
the numbers of characters written (or dropped in the no canon_data case). 
Relying on receive_room value instead is rather bogus.

Below is a new patch, which also fixes problems with very long lines.
1. if receive_room() returns a small number, write_chan() has to 
continue writing, otherwise it will sleep with nobody waking it up.
2. we mustn't simply drop eol characters in n_tty_receive_char otherwise 
canon_data isn't increased and the reader isn't woken up.

bye, Roman

 n_tty.c |   33 +++++++++++++++------------------
 1 files changed, 15 insertions(+), 18 deletions(-)

Index: linux-2.6/drivers/char/n_tty.c
===================================================================
--- linux-2.6.orig/drivers/char/n_tty.c	2005-02-16 16:01:35.000000000 +0100
+++ linux-2.6/drivers/char/n_tty.c	2005-02-16 19:17:13.053000548 +0100
@@ -770,10 +770,8 @@ send_signal:
 		}
 		if (c == '\n') {
 			if (L_ECHO(tty) || L_ECHONL(tty)) {
-				if (tty->read_cnt >= N_TTY_BUF_SIZE-1) {
+				if (tty->read_cnt >= N_TTY_BUF_SIZE-1)
 					put_char('\a', tty);
-					return;
-				}
 				opost('\n', tty);
 			}
 			goto handle_newline;
@@ -790,10 +788,8 @@ send_signal:
 			 * XXX are EOL_CHAR and EOL2_CHAR echoed?!?
 			 */
 			if (L_ECHO(tty)) {
-				if (tty->read_cnt >= N_TTY_BUF_SIZE-1) {
+				if (tty->read_cnt >= N_TTY_BUF_SIZE-1)
 					put_char('\a', tty);
-					return;
-				}
 				/* Record the column of first canon char. */
 				if (tty->canon_head == tty->read_head)
 					tty->canon_column = tty->column;
@@ -862,12 +858,9 @@ static int n_tty_receive_room(struct tty
 	 * that erase characters will be handled.  Other excess
 	 * characters will be beeped.
 	 */
-	if (tty->icanon && !tty->canon_data)
-		return N_TTY_BUF_SIZE;
-
-	if (left > 0)
-		return left;
-	return 0;
+	if (left <= 0)
+		left = tty->icanon && !tty->canon_data;
+	return left;
 }
 
 /**
@@ -1473,13 +1466,17 @@ static ssize_t write_chan(struct tty_str
 			if (tty->driver->flush_chars)
 				tty->driver->flush_chars(tty);
 		} else {
-			c = tty->driver->write(tty, b, nr);
-			if (c < 0) {
-				retval = c;
-				goto break_out;
+			while (nr > 0) {
+				c = tty->driver->write(tty, b, nr);
+				if (c < 0) {
+					retval = c;
+					goto break_out;
+				}
+				if (!c)
+					break;
+				b += c;
+				nr -= c;
 			}
-			b += c;
-			nr -= c;
 		}
 		if (!nr)
 			break;
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/