Subject: tty ioctl() proposal
To: None <tech-kern@NetBSD.ORG>
From: Charles M. Hannum <mycroft@mit.edu>
List: tech-kern
Date: 02/22/1998 07:20:29
First, let's review how tty ioctl()s are handled.

We indirect through some magic (the nature of which is not relevant to
the following discussion) to the device driver; e.g. comioctl().  In
the device driver, we have a piece of code that calls the line
discipline and the generic tty layer to handle `standard' ioctl()s.
The way the line discpline and ttioctl() tell us to try processing an
ioctl() through a different layer is by returning -1.  So the code
currently looks something like:

        error = (*linesw[tp->t_line].l_ioctl)(tp, cmd, data, flag, p);
        if (error >= 0)
                return (error);

        error = ttioctl(tp, cmd, data, flag, p);
        if (error >= 0)
                return (error);

If both of these calls failed, then we fall through and handle our own
specialized ioctl()s, or generic ones that have device-specific
implemenations.  Something like this:

	switch (cmd) {
	case TIOCMGET:
		...
	...
	}


Now here's the weird part.

In my recent change to the com driver, I made this latter part look
like:

	s = splserial();

	switch (cmd) {
	case TIOCMGET:
		...
	...
	default:
		error = ENOTTY;
		break;
	}

	splx(s);

	return (error);

The problem is that I forgot to reset `error' to 0, so in the case of
successful ioctl()s (like TIOCMGET), it was never changed after the
call to ttioctl(), and comioctl() would return -1.  This of course
happens to be ERESTART, and the omission therefore caused the system
call to be restarted ad infinitum (though it could be interrupted with
an unblocked signal).


While this is certainly easy enough to fix, and it's arguable that
this is a similar case to an uninitialized variable, I think it's kind
of poor to use -1 (or, rather, ERESTART) in this case.  So, I propose
creating another magic error value, which for lack of a better name I
temporarily dub `ENOTHANDLED', for use in this situation.  Device
driver ioctl() routines would then test the return value with `!=
ENOTHANDLED' rather than `>= 0'.  In addition to making some potential
lossage modes (like the one mentioned above) easier to diagnose, this
would also allow ioctl()s that block to be properly interrupted and
restarted, per SA_RESTART and all that jazz.  (I'm not currently aware
of any case where that would be useful, but you never know...)


Any comments from the cashew gallery?