smk@axiom.UUCP (Steven M. Kramer) (02/26/84)
The variable allflags in main.c of the 4.2 getty (which is excellent
in most respects and quite an improvement over the older
non-tabularized getty's) assumes the bits for te TIOCLSET and the bits
for the st_flags word of TIOCSETP word are distinct. They never were
before but now they are. Porting getty to older version of UNIX will
cause a mess because of this. My solution is to separate setflags()
in subr.c into two routines: setflags() and setlocalmode(). The
diffs below show the solution. Note that I'm passing a short to
the TIOCLSET ioctl in main.c and this may not work on your UNIX (like
4.xBSD). To fix that, make allflags a long and the result will work
much more portabally than before.
Hey, am I right on the delay code? It seems 0 is supposed
to give the ??0 setting. Any other changes along those lines?
--steve kramer
--------------------------
*** /usr/src/etc/getty/main.c Mon Aug 1 19:07:07 1983
--- main.c Sat Feb 25 17:51:39 1984
***************
*** 91,96
char *argv[];
{
char *tname;
long allflags;
signal(SIGINT, SIG_IGN);
--- 99,119 -----
char *argv[];
{
char *tname;
+ #ifdef AXIOM
+ /*
+ * On our port, the kernel does a sizeof to find out the
+ * size of the buffer. Since it is a short, specifying a
+ * long essentially always zeroes the setting (for the
+ * TIOCL[GS]ET cases. We make it unsigned because the top
+ * bit can be set which may hurt at some point. We also
+ * declare setflags() and setlocalmode() to be unsigned
+ * short to properly reflect what's going on here.
+ * Steve Kramer 2/24/84
+ */
+ unsigned short allflags;
+ extern unsigned short setflags();
+ extern unsigned short setlocalmode();
+ #else
long allflags;
#endif
***************
*** 92,97
{
char *tname;
long allflags;
signal(SIGINT, SIG_IGN);
/*
--- 115,121 -----
extern unsigned short setlocalmode();
#else
long allflags;
+ #endif
signal(SIGINT, SIG_IGN);
/*
***************
*** 153,158
continue;
allflags = setflags(2);
tmode.sg_flags = allflags & 0xffff;
allflags >>= 16;
if (crmod || NL)
tmode.sg_flags |= CRMOD;
--- 177,195 -----
continue;
allflags = setflags(2);
tmode.sg_flags = allflags & 0xffff;
+ #ifdef AXIOM
+ /*
+ * The original code here was bit dependent and
+ * assumed that the bits for two separate functions
+ * (the mode and local mode setting) did not
+ * collide. This is only true on 4.2BSD. We
+ * solve the problem on other systems (like our 68000
+ * one) by making two separate routines deal with
+ * the bits.
+ * Steve Kramer 2/24/84
+ */
+ allflags = setlocalmode();
+ #else
allflags >>= 16;
#endif
if (crmod || NL)
***************
*** 154,159
allflags = setflags(2);
tmode.sg_flags = allflags & 0xffff;
allflags >>= 16;
if (crmod || NL)
tmode.sg_flags |= CRMOD;
if (upper || UC)
--- 191,197 -----
allflags = setlocalmode();
#else
allflags >>= 16;
+ #endif
if (crmod || NL)
tmode.sg_flags |= CRMOD;
if (upper || UC)
*** /usr/src/etc/getty/subr.c Thu Jul 7 06:32:55 1983
--- subr.c Sat Feb 25 18:04:26 1984
***************
*** 114,119
}
}
long
setflags(n)
{
--- 122,139 -----
}
}
+ #ifdef AXIOM
+ /*
+ * We define the routine here and local variables to be what they
+ * REALLy are, not any more or less. In dealing with bit positions
+ * and arguments to ioctl()'s, this is crucial.
+ * Steve Kramer 2/24/84
+ */
+ unsigned short
+ setflags(n)
+ {
+ register unsigned short f;
+ #else
long
setflags(n)
{
***************
*** 118,123
setflags(n)
{
register long f;
switch (n) {
case 0:
--- 138,144 -----
setflags(n)
{
register long f;
+ #endif
switch (n) {
case 0:
***************
*** 162,167
if (!HT)
f |= XTABS;
if (n == 0)
return (f);
--- 183,215 -----
if (!HT)
f |= XTABS;
+ #ifdef AXIOM
+ /*
+ * NERTZ! They changed the way 4.2BSD does mode setting but
+ * forgot to say so in the documentation. They changed the definition
+ * of the local mode bits so they wouldn't collide with those
+ * of the sg_flags bits. Then, they used the same routine to
+ * set all the bits at once. The problem is that this is very
+ * inplementation dependent and it stinks. It should be, and is,
+ * two routines in our implementation. Note that the ECHO mode
+ * setting belongs here.
+ * Steve Kramer 2/25/84
+ */
+
+ if (EC)
+ f |= ECHO;
+
+ return (f);
+ }
+
+
+ unsigned short
+ setlocalmode()
+ {
+ register unsigned short f;
+
+ f = 0L;
+ #else
if (n == 0)
return (f);
***************
*** 165,170
if (n == 0)
return (f);
if (CB)
f |= CRTBS;
--- 213,219 -----
if (n == 0)
return (f);
+ #endif
if (CB)
f |= CRTBS;
***************
*** 174,179
if (PE)
f |= PRTERA;
if (EC)
f |= ECHO;
--- 223,235 -----
if (PE)
f |= PRTERA;
+ #ifdef AXIOM
+ /*
+ * Move this to the setflags() routine. It conceptually and
+ * now definitely belongs there.
+ * Steve Kramer 2/25/84
+ */
+ #else
if (EC)
f |= ECHO;
***************
*** 177,182
if (EC)
f |= ECHO;
if (XC)
f |= CTLECH;
--- 233,239 -----
if (EC)
f |= ECHO;
+ #endif
if (XC)
f |= CTLECH;
***************
*** 198,203
3, CR3,
83, CR1,
166, CR2,
0, CR3,
};
--- 255,267 -----
3, CR3,
83, CR1,
166, CR2,
+ #ifdef AXIOM
+ /*
+ * 0 SHOULD mean CR0 obviously.
+ * Steve Kramer 2/22/84
+ */
+ 0, CR0,
+ #else
0, CR3,
#endif
};
***************
*** 199,204
83, CR1,
166, CR2,
0, CR3,
};
struct delayval nldelay[] = {
--- 263,269 -----
0, CR0,
#else
0, CR3,
+ #endif
};
struct delayval nldelay[] = {
***************
*** 206,211
2, NL2,
3, NL3,
100, NL2,
0, NL3,
};
--- 271,283 -----
2, NL2,
3, NL3,
100, NL2,
+ #ifdef AXIOM
+ /*
+ * 0 SHOULD mean NL0 obviously.
+ * Steve Kramer 2/22/84
+ */
+ 0, NL0,
+ #else
0, NL3,
#endif
};
***************
*** 207,212
3, NL3,
100, NL2,
0, NL3,
};
struct delayval bsdelay[] = {
--- 279,285 -----
0, NL0,
#else
0, NL3,
+ #endif
};
struct delayval bsdelay[] = {
***************
*** 217,222
struct delayval ffdelay[] = {
1, FF1,
1750, FF1,
0, FF1,
};
--- 290,302 -----
struct delayval ffdelay[] = {
1, FF1,
1750, FF1,
+ #ifdef AXIOM
+ /*
+ * 0 SHOULD mean FF0 obviously.
+ * Steve Kramer 2/22/84
+ */
+ 0, FF0,
+ #else
0, FF1,
#endif
};
***************
*** 218,223
1, FF1,
1750, FF1,
0, FF1,
};
struct delayval tbdelay[] = {
--- 298,304 -----
0, FF0,
#else
0, FF1,
+ #endif
};
struct delayval tbdelay[] = {
--
--steve kramer
{allegra,genrad,ihnp4,utzoo,philabs,uw-beaver}!linus!axiom!smk (UUCP)
linus!axiom!smk@mitre-bedford (MIL)kre@mulga.SUN (Robert Elz) (02/27/84)
To: Steven M Kramer @ Axiom (posted as a correction of earlier news) I'm glad that you approve of the 4.2bsd getty (which is one of my small contributions), however, your bug fixes really just aren't ... Easiest is your "fix" to the delay setting tables. There you simply have not read the code. If you look in adelay() in subr.c, you will see that the delay tables are searched in ascending order of delay values, stopping when a delay greater than, or equal to, the requested delay is encountered. To stop such a search, I have used a '0' as a sentinal value, a fairly common technique I think. An alternative would be to use a value bigger than anything anyone would ever request - but how big is that? The '0' means "anything bigger than the previous value", and should obviously result in the longest possible delay, not none! Adelay() has a special case for a requested delay of 0. The "portability" bug in the handling of the flag bits is really just an implementation technique, perhaps not the best possible, but not really a bug either. Getty uses the new, 4.2bsd, #defines of CRTBS, CTLECH, etc, which aren't available on previous systems (really they're not intended for use outside the kernel on 4.2, but getty is a pretty special program). These names are defined to be (LCRTBS<<16) (etc) - they must have 16 low order zero bits, and cannot conflict with any of the sg_flags bits used with TIOCSETP. If you are going to port getty from 4.2 to some other system, you should port the #defines as well, I guess that you have assumed that CRTBS is just a new name for LCRTBS; such is not the case, LCRTBS is still there, and has the same value that it always had. It hasn't been "moved". It is true, as you suggest, that the code would be a little cleaner if the "standard" flags and the "local" flags were handled by distinct routines - that is while the distinction between them remains, which it didn't on 4.1c bsd, where the 4.2 getty was developed. Its quite likely that sometime the distinction will vanish again, after all, they're all just flags ... Robert Elz decvax!mulga!kre ps: just in case anyone hasn't guessed, I'm suggesting that you ignore the suggested fixes to getty, and leave it as it is.