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 [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
