On Wed, 16 Feb 2005, Roman Zippel wrote:
>
> The patch below seems to do the trick too.
> It seems the initial receive_room() call in pty_write() returns
> N_TTY_BUF_SIZE and receive_buf() will happily drop the last byte.
Why have that "tty->icanon && !tty->canon_data" test in the first place, I
wonder? Isn't the "left" calculation always correct? That's really how
many bytes free we have in the tty, that "canon_data" thing is just about
how much of it is available for _reading_ as canon, no? (Ie "how many
characters that have seen a finishing end-of-line"). So I don't see why
that canon_data test is relevant to the question of filling the buffer..
In particular, afaikt, "canon_data" can be zero even if we _have_
characters in the queue (just not aany EOLN), so I'd expect your version
to still return "N_TTY_BUF_SIZE-1" even though the queue can only actually
take fewer characters..
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.. (In
contrast, changing the chunking to 2kB not only changes a "off-by-one" to
a "off-by-2048", but more importantly is what we've tested for the last
decade or so ;)
In fact, I _think_ that the whole
if (tty->icanon && !tty->canon_data)
test is about the fact that we _want_ the writer to start dropping
characters at some point, since if we're in canon mode, we need to get a
full line in the end, and if there is no EOLN to be had, then we _need_ to
drop characters.
So I'd think that the n_tty_receive_room() function should look something
like this:
static int n_tty_receive_room(struct tty_struct *tty)
{
int left = N_TTY_BUF_SIZE - tty->read_cnt - 1;
if (left <= 0)
left = tty->icanon && !tty->canon_data;
return left;
}
which basically says: only accept as much data as we can fit in the
buffer, BUT if we can't really fit anything and we're in canon mode (but
haven't seen a newline yet), we need to allow the writer to continut to
trickle data (that we will discard) in the hopes of _eventually_ seeing an
EOLN.
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?
Linus
-
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/
