[lkml]   [2009]   [Sep]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
SubjectRe: [Bug #14015] pty regressed again, breaking expect and gcc's testsuite

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

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


 \ /
  Last update: 2009-10-18 23:28    [from the cache]
©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site