svn commit: r311952 - head/sys/ddb

[Mark Johnston]

On Fri, Jan 13, 2017 at 02:51:04PM +1100, Bruce Evans wrote:
> On Thu, 12 Jan 2017, Mark Johnston wrote:
> 
> > Log:
> >  Enable the use of ^C and ^S/^Q in DDB.
> >
> >  This lets one interrupt DDB's output, which is useful if paging is
> >  disabled and the output device is slow.
> 
> This is quite broken.  It removes the code that was necessary for avoiding
> loss of input.

Hi Bruce,

I've reverted this for now. I haven't thought much about how
db_check_interrupt() might losslessly poll the console driver for
ctrl-C, but I do think it's a valuable feature to have - just this
morning I absent-mindedly dumped a 128K-entry KTR ring from a DDB prompt
after having set $lines = 0, and had to wait for quite a while to get
my prompt back.

> 
> > Modified: head/sys/ddb/db_input.c
> > ==============================================================================
> > --- head/sys/ddb/db_input.c	Thu Jan 12 00:09:31 2017	(r311951)
> > +++ head/sys/ddb/db_input.c	Thu Jan 12 00:22:36 2017	(r311952)
> > @@ -63,7 +63,6 @@ static int	db_lhist_nlines;
> > #define	BLANK		' '
> > #define	BACKUP		'\b'
> >
> > -static int	cnmaygetc(void);
> > static void	db_delete(int n, int bwd);
> > static int	db_inputchar(int c);
> > static void	db_putnchars(int c, int count);
> > @@ -291,12 +290,6 @@ db_inputchar(c)
> > 	return (0);
> > }
> >
> > -static int
> > -cnmaygetc()
> > -{
> > -	return (-1);
> > -}
> > -
> 
> BSD never had a usable console API (function) for checking for input.
> cncheckc() is the correct name for such a function.  The actual
> cncheckc() returns any input that it finds.  This is not the main
> problem here.  The input will have to be read here anyway to determine
> what it is.  The problems are that there is no console API at all for
> ungetting characters in the input stream, and this function doesn't
> even use a stub cnungetc(), but drops the input on the floor.  It does
> document this behaviour in a comment, but doesn't say that it is wrong.
> 
> > int
> > db_readline(lstart, lsize)
> > 	char *	lstart;
> > @@ -350,7 +343,7 @@ db_check_interrupt(void)
> > {
> > 	int	c;
> >
> > -	c = cnmaygetc();
> > +	c = cncheckc();
> > 	switch (c) {
> > 	    case -1:		/* no character */
> > 		return;
> 
> The stub function always returns -1, so db_check_interrupt() is turned into
> a no-op.
> 
> db_check_interrupt() now eats the first byte of any input on every call.
> 
> > @@ -361,7 +354,7 @@ db_check_interrupt(void)
> >
> > 	    case CTRL('s'):
> > 		do {
> > -		    c = cnmaygetc();
> > +		    c = cncheckc();
> > 		    if (c == CTRL('c'))
> > 			db_error((char *)0);
> > 		} while (c != CTRL('q'));
> 
> This is now a bad implementation of xon/xoff.  It doesn't support ixany,
> but busy-waits for ^Q or ^C using cncheckc().  It should at least
> busy-wait using cngetc(), since that might be do a smarter busy wait.
> cngetc() is actually only slightly smarter -- it uses busy-waiting too,
> but with cpu_spinwait() in the loop.  This cpu_spinwait() makes little
> difference since cncheckc() tends to be a heavyweight function.  Only
> cpu_spinwait()s in the innermost loops of cncheckc(), if any, are likely
> to have much effect.
> 
> Multiple consoles complicate this a bit.
> 
> db_check_interrupt() is called after every db_putc() of a newline.  So
> the breakage is mainly for type-ahead while writing many lines.

I'll note that type-ahead interacts rather poorly with DDB's behaviour
of repeating the previous command upon reading an empty input line:
entering a carriage return at any point while "show ktr" is working will
cause it to be started again once it's finished.

> 
> Control processing belongs in the lowest level of console drivers, and
> at least the xon/xoff part is very easy to do.  This makes it work for
> contexts like boot messages -- this can only be controlled now by booting
> with -p, which gives something like an automatic ^S after every line,
> but the control characters for this mode are unusual and there is no
> way to get back to -p mode after turning it off or not starting with it.
> 
> [...]