Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 17 Feb 2012 01:38:23 +1100 (EST)
From:      Bruce Evans <brde@optusnet.com.au>
To:        Bruce Evans <brde@optusnet.com.au>
Cc:        svn-src-head@freebsd.org, Martin Cracauer <cracauer@freebsd.org>, Martin Cracauer <cracauer@cons.org>, src-committers@freebsd.org, svn-src-all@freebsd.org
Subject:   Re: svn commit: r231449 - head/usr.bin/tee
Message-ID:  <20120217012142.I2506@besplex.bde.org>
In-Reply-To: <20120212040658.D4220@besplex.bde.org>
References:  <201202102216.q1AMGI0m098192@svn.freebsd.org> <20120211194854.J2214@besplex.bde.org> <20120211163807.GA25525@cons.org> <20120212040658.D4220@besplex.bde.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Sun, 12 Feb 2012, Bruce Evans wrote:

> BTW, one of the many bugs in the tty driver in -current is that it no
> longer does watermark processing for select() and poll(), so it reads
> and writes tinygrams even when polled using select() and poll() (and
> there is no better way).  I use the following quick fix:
>
> % Index: ttydisc.h
> % ===================================================================
> % RCS file: /home/ncvs/src/sys/sys/ttydisc.h,v
> % retrieving revision 1.7
> % diff -u -2 -r1.7 ttydisc.h
> % --- ttydisc.h	23 Aug 2009 08:04:40 -0000	1.7
> % +++ ttydisc.h	25 Sep 2010 14:37:54 -0000
> % @@ -70,8 +70,13 @@
> %  ttydisc_read_poll(struct tty *tp)
> %  {
> % +	size_t navail;
> % %  	tty_lock_assert(tp, MA_OWNED);
> % % -	return ttyinq_bytescanonicalized(&tp->t_inq);
> % +	navail = ttyinq_bytescanonicalized(&tp->t_inq);
> % +	if ((tp->t_termios.c_lflag & ICANON) == 0 &&
> % +	    navail < tp->t_termios.c_cc[VMIN] && tp->t_termios.c_cc[VTIME] == 
> 0)
> % +		navail = 0;
> % +	return (navail);
> %  }
> % % @@ -79,8 +84,10 @@
> %  ttydisc_write_poll(struct tty *tp)
> %  {
> % +	size_t nleft;
> % %  	tty_lock_assert(tp, MA_OWNED);
> % % -	return ttyoutq_bytesleft(&tp->t_outq);
> % +	nleft = ttyoutq_bytesleft(&tp->t_outq);
> % +	return (nleft >= tp->t_outlow ? nleft : 0);
> %  }
> %
>
> The watermarks that affect applications should be under control of the
> application like they are for sockets.  There is only limited control
> of the read watermark for ttys, by enabling MIN and setting it as high
> as possible (the maximum is normally UCHAR_MAX which is normally 255),
> and this is not standardized, but it works in Linux and used to work
> in FreeBSD.
>
> The watermarks that affect drivers should be under control of drivers
> like they used to be.

Here is a program that demonstrates the brokenness of select() on any
tty in -current (I tested with an ssh pty).  select() is specified to
not return before i/o can be done without blocking.  But -current
blocks.

This program uses blocking mode so that POSIX is clearly violated.
Practical programs need to use non-blocking mode so they don't block
if something steals their input, or if there is a kernel bug like this.
Then they need select() to not return before i/o can be done wthout
blocking _in blocking mode_ although they actually use non-blocking 
mode to do the i/o.  Otherwise they will do i/o in tinygrams.  I just
checked POSIX and was a little surprised to find that it specifies
the correct behaviour perfectly: "A descriptor shall be considered
ready for reading when a call to an input function with O_NONBLOCK
clear would block..."

% #include <err.h>
% #include <fcntl.h>
% #include <limits.h>
% #include <stdio.h>
% #include <stdlib.h>
% #include <termios.h>
% #include <unistd.h>
% 
% int 
% main(void)
% {
% 	fd_set readfds;
% 	struct termios ot, t;
% 	int flags, n;
% 	char buf[8];
% 
% 	flags = fcntl(0, F_SETFL);
% 	if (flags == -1)
% 		err(1, "fcntl");
% 	if (fcntl(0, F_SETFL, flags & ~O_NONBLOCK) != 0)
% 		err(1, "fcntl");
% 	if (tcgetattr(0, &t) != 0)
% 		err(1, "tcgetattr() of stdin");
% 	ot = t;
% 	cfmakeraw(&t);
% 	t.c_cc[VMIN] = 2;
% 	t.c_cc[VTIME] = 0;
% 	if (tcsetattr(0, TCSANOW, &t) != 0)
% 		err(1, "tcsetattr() of stdin");
% 	FD_SET(0, &readfds);
% 	fprintf(stderr, "Type 2 letters.  This should not proceed\r\n");
% 	fprintf(stderr, "to `select returned' after only 1 ... ");
% 	if (select(1, &readfds, NULL, NULL, NULL) <= 0)
% 		err(1, "select");
% 	fprintf(stderr, "select returned ... ");
% 	n = read(0, buf, sizeof(buf));
% 	fprintf(stderr, "read returned `");
% 	if (n < 0)
% 		err(1, "read");
% 	if (tcsetattr(0, TCSANOW, &ot) != 0)
% 		err(1, "tcsetattr() of stdin");
% 	(void)write(1, buf, n);
% 	fprintf(stderr, "'\n");
% 	exit(0);
% }

The error handling here is excessive but still not complete.  If fails
to restore the terminal state after an err() exit.  "stty sane" also
fails to restore the terminal to is sane state.  It leaves MIN at 2,
which can be very confusing.  Most shells change it to 1 for their
editor, then restore it to 2 to confuse the next program that doesn't
change it.

Bruce



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20120217012142.I2506>