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 kernel-testers" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
