Re: [PATCH] kdesu broken

From: Linus Torvalds
Date: Tue Jul 28 2009 - 11:48:26 EST




On Tue, 28 Jul 2009, OGAWA Hirofumi wrote:
>
> Just a quick hack though. Is this wrong/unpreferable way?

That seems to be right regardless of any other issues.

> n_tty_read() checks the pending buffer and consume it before
> input_available_p().

Why not move this _inside_ "input_available_p()"? There are only two
call-sites, and strictly speaking they both want it.

Look at n_tty_poll(), for example:

if (input_available_p(tty, TIME_CHAR(tty) ? 0 : MIN_CHAR(tty)))
mask |= POLLIN | POLLRDNORM;
if (tty->packet && tty->link->ctrl_status)
mask |= POLLPRI | POLLIN | POLLRDNORM;
if (test_bit(TTY_OTHER_CLOSED, &tty->flags))
mask |= POLLHUP;
if (tty_hung_up_p(file))
mask |= POLLHUP;

and notice what happens to somebody who uses select/poll when there might
be pending data that hasn't been handled yet, and the tty has been marked
TTY_OTHER_CLOSED or hung up. It would return only POLLHUP, and as a
result, that side would never even try to read the pending data because
poll implies that there is no data and it's EOF. Which is just wrong.

So _any_ time you check "is there input available?" you should always
check if there are other buffers. No?

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/