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

From: OGAWA Hirofumi
Date: Sat Sep 05 2009 - 14:56:18 EST

Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> writes:

> 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.

But, current write_room() returns almost all wrong value. For example,
if we have the 4kb preallocated buffer in some state and used it,
->memory_used will be 4kb even if we are using only a byte actually.

I thought it's strange/wrong, even if we removed the pty_space() in

>> 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.

As a bit long term solution, I agree. Current code seems to have fragile
buffer handling about echoes, \n etc. And yes, perhaps, to avoid
write_room() is clean way.

But, I felt 64kb (pty_write) vs 8kb (pty_write_room) sounds strange

OGAWA Hirofumi <hirofumi@xxxxxxxxxxxxxxxxxx>
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at
Please read the FAQ at