Re: [Bug #14015] pty regressed again, breaking expect and gcc'stestsuite

[Linus Torvalds]

On Sun, 6 Sep 2009, OGAWA Hirofumi wrote:
> 
> This is not meaning to object to your patch though, I think we would be
> good to fix pty_space(), not leaving as wrong. With fix it, I guess we
> don't get strange behavior in the near of buffer limit.

I'd actually rather not make that function any more complicated.

Just make the rules be very simple:

 - the pty layer has ~64kB buffering, and if you just blindly do a 
   ->write() op, you can see how many characters you were able to write.

 - before doing a ->write() op, you can ask how many characters you are 
   guaranteed to be able to write by doing a "->write_room()" call.

..and then the bug literally was just that "pty_write()" was confused, and 
thought that it should do that "write_room()" thing, which it really 
shouldn't ever have done.

So I really think that the true fix is to just remove the code from 
pty_write(), and not do anything more complicated. I'll also commit the 
change to write '\r\n' as one single string, because quite frankly, it's 
just stupid to do it as two characters, but at that point it's just a 
cleanup.

> Also, it seems the non-n_tty path doesn't use tty_write_room() check,
> and instead it just try to write and check written bytes which returned
> by tty->ops->write().

.. and I think that's fine. I think write_room() should be used sparingly, 
and only by code that cares about being able to fit at least 'n' 
characters in the tty buffers. In fact, I think even n_tty would likely in 
general be better off without it (and just check the return value), but 
because of the stateful character translation (that doesn't actually keep 
any state around, it just wants to expand things as it goes along), and 
because of historical reasons, we'll just keep it using write_room.

		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/