[comp.os.minix] Bug in fs/device.c, ioctl.c, tty.c

des@berlioz.nsc.com (Desmond Young) (03/28/89)

Guday,
  In the course of adding an ethernet driver to Minix I have come across
a bug in ioctl.c, fs/device.c and tty.c. This is introduced when you
change the speed on your tty line. I think I remember some atari
complaints about speed changes. Here is your problem.
  Skip this paragraph if you are familiar with the code. When you do I/O
the library routines build a message, and send it to fs. Fs munges on
the parameters, passes the appropriate stuff to the driver, and waits.
The driver does its stuff, sends a message back to fs, which replies to
your (user) program.
  So, here is the problem. Whoever added the speed stuff to the tty code
introduced a subtle bug, (and cost me quite a bit of time due to their
laziness, not the bug). My UNIX manuals say for ioctl the user passes
a pointer to the sgttyb (or terminfo etc.) structure. In Minix, this is
not so. Instead, ioctl.c packs the parameters into ints and longs in the
message to fs. Fs then copies them from the user's message into its
message to tty.c. Two problems:
  1. It does not copy over all the fields (LAZY PROGRAMMING). I WAS
     passing a struct pointer for my ethernet driver, it never got there.
  2. The person who added the TTY_SPEED assignment defined it as m2.i3
     (3rd int of message type 2). THIS IS THE FIELD USED TO STORE THE PROC
     NUMBER (OH SHIT). What happens is this. Fs sits in a loop something
     like this:
	while (m->PROC_NR != proc)
		process somebody elses return from a driver;
     That is: you request an ioctl (or read/write). Fs puts your proc number
     into the m.PROC_NR field of the message. This has two purposes, first
     the tty driver uses it to figure out the core address for a direct
     transfer to user space (read/writes), second, the driver returns this
     field to fs so it knows which proc has been serviced.

SO, we get to the meat. When you set a non-zero speed, fs puts the speed into
your message's PROC_NR field. The tty driver does its stuff (for ioctl's it
ignores the proc number). The driver copies the speed number back as the
proc number and sends a message to fs. Fs says "oh, I can reply to proc(speed)
now". But unless speed=your_proc_number, shit.
  So you say, how come getty/login works? Well, you put say, 5 in the speed.
Fs sets proc=5, driver answers completed 5, so fs wakes you up (thinking all
along you were proc 5). BUT if there was a real proc 5 running ???
  Anyway, for my ethernet driver I really pass a struct pointer. And I have
changed my code for tty driver to pass a struct. This has the added benefit
that I can add fields to sgttyb struct in the future (Minix has filled up
the message, so cannot add any more).
  I find the Minix kernel so awfully slow for device interrupts etc. that I
do not want to keep compatibility, thus I have a non-standard code set. If
enough people want, I could post my fixes as diffs against 1.3c/d. However,
my ioctl supports FIONREAD for non-blocking I/O (thanks to Ed Hall), and
supports an ethernet controller (mine). Both were added to support TCP-IP.
  Perhaps some kind person will post the fixes, ioctl.c, fs/device.c, and
tty.c.
Cheers, Des.

Reply:  des@logic.nsc.com
       {decwrl,hplabs,sun,pyramid,amdahl}!nsc.com!logic!des
Des Young, M/S D3635 National Semiconductor, Santa Clara
The light at the end of the tunnel was only a burglar's torch - J.Buffet.

bds@lzaz.ATT.COM (B.SZABLAK) (03/28/89)

In article <11714@louie.udel.EDU>, des@berlioz.nsc.com (Desmond Young) writes:
>   2. The person who added the TTY_SPEED assignment defined it as m2.i3
>      (3rd int of message type 2). THIS IS THE FIELD USED TO STORE THE PROC
>      NUMBER (OH SHIT)...

My biggest complaint about the way MINIX is programmed is this overloading
of message fields. It is confusing and error prone (at least when modifing
things. I would have prefered a more "natural" approach:

struct { int msgtype; union { struct type1 m1; ...; struct typeN mN; }; };

I can appreciate the problems with such an approach, but for an
"instructional" OS, I believe this is a better way of doing things
since it is clearer and less error prone (a good thing even for "real" OSs).