Date: Thu, 23 Feb 1995 07:37:42 -0800 From: Paul Traina <pst@shockwave.com> To: "Ugen J.S.Antsilevich" <ugen@netvision.net.il> Cc: current@FreeBSD.org Subject: Re: snp(4)/watch(8) code review comments Message-ID: <199502231537.HAA16439@precipice.Shockwave.COM> In-Reply-To: Your message of "Thu, 23 Feb 1995 16:55:56 %2B0700." <Chameleon.950223170655.ugen@ugen.NetManage.co.il>
next in thread | previous in thread | raw e-mail | index | archive | help
From: "Ugen J.S.Antsilevich" <ugen@NetVision.net.il>
Subject: Re: snp(4)/watch(8) code review comments
>I want to add a new field to cdevsw[] that is a vector to a function in the
>driver that will return a pointer to the right struct tty element given a
>major/minor number. This sort of decision needs to be done locally to the
>driver to account for things like major/minor number masking (e.g. cua0 vs
>ttyd1 in the sio driver).
>Does anyone have an objection to my adding this new field? It will simply
>be NULL for any device not supporting that vector and all invocations of it
>test for non-NULL first before calling.
Hmm...add the whole function just for such a thing like snoop...Too much:)
Why? It's clearly the most os/driver/architecture independant change and
it's something that we kludge around elsewhere in the system too (for example,
ttselect is wrappered in most drivers specificly to handle the minor number
conversion brain-dammage).
Actually why can't you just use minor number as index in tty table already
in cdevsw?? ok..i know that you'd have a problemm about serial lines with
their weird minor number stuff but on the other hand you can put conversion
of minor number to index in table as a part of watch utility..you anyway need
It doesn't belong in a user program, bite your tounge! The whole reason
I was doing this was to remove evil stuff like that from watch. :-)
Also, if we just index off the array, there's nothing that stops root from
specifying an illegal device and crashing the system because we start
corrupting kernel data space if you give a minor number that's too large.
some conversion if you want (at least I want:))) to use not only normal
device names like ttyp0 but also pty0,vty0 and other symbolic names...
That's exactly what we get if we do this right, since each driver knows
the relationship between minor number and tty structure already and that
changes from driver to driver (e.g. if I rip out sio and install comm,
major/minor number splitting changes, but the names, like cua0/ttyd1 don't).
Major change to cdevsw is may be nice but too big...:)) *IMHO*
It's not like we couldn't use it elsewhere. I'm not against doing architectual
things if they implement something cleanly.... and adding an option vector is
hardly a major change... just the core kernel changes.
>Actually, I'm just doing it on general principles, I never actually intend
>to use the snp drivers in real life, I was just digging through things and
>didn't like the current interface so I started hacking on it.
>
>If you want, I can feed you what I've done so far, or just finish it up
>this weekend.
OK, I'm going to keep on going with what I've started and see what the total
change is when I'm done. Right now, I'd much rather add an optional vector
to cdevsw than make snp have to know about every single tty driver out there
(for instance, you missed the tw and cy drivers, which this covers in an
driver-independant fashion :-))
Hmm.if you already doing this-continue,it's ok with me 100% but better
don't do that change to cdevsw-this still can be done another less radical
ways.
I'm more than open for suggestions for other ways to do this cleanly, using
cdevsw was the first thing I thought of, but if anyone else on the team has
a good idea here, I'll go for it.
Paul
p.s. yes, no problem, I'll go through and work on your ipfw man pages too,
it shouldn't be too bad, it only took me about 20 minutes to do snp(4)
and watch(8)... using mandoc is really easy once you do your first couple
of pages... I wish I knew where the official guide to mandoc was in the
FreeBSD source code, it seems we have lost it?
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?199502231537.HAA16439>