Re: Pty is losing bytes

[Linus Torvalds]

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