laman@ncr-sd.UUCP (Mike Laman) (10/31/85)
I received several requests for my bug fxes, so here they are. I'm sure
others will find them interesting.
Here is a list of bug fixes I sent out on the net abot a year ago for the
5.2 terminfo. These should also apply to the 5.2.2 terminfo curses library,
too.
5.2CursesFix1
There are two bugs in the "portable" routine "getsh()" in the file
screen/setupterm.c. The first bug is that the comparison "if (*p == 0377)"
is incorrect if characters are signed by default. If *p contained the
BYTE value '\377', sign extension would make it so *p != 0377 since 0377
is an integer and will NOT be sign extended.
The second bug is that "rv = *p++" will do sign extension (for systems with
characters being signed). What we want is to sign extend on ONLY the
"high part"; more precisely, we want to sign extend ONLY on the assignment
that follows which puts the high byte in and NOT on the assignment that puts
the low byte in.
[ I suspect that if this routine was used, it was used on a machine where
characters are unsigned. ]
The original getsh() follows:
getsh(p)
register char *p;
{
register int rv;
if(*p == 0377)
return -1;
rv = *p++;
rv += *p * 256;
return rv;
}
the following should do the trick:
getsh(p)
register char *p;
{
register int rv;
rv = *((unsigned char *) p);
if(rv == 0377)
return -1; /* This is a pretty common case */
return rv + (*++p * 256);
}
Instead of this though, I use the following macro (since this is for a machine
with signed characters):
#define getsh(ip) (*((unsigned char *) ip) | (*(ip+1) << 8))
Folks on machines with unsigned character implementations could use:
#define getsh(ip) ((*ip == 0377) ? -1 : (*ip | (*(ip+1) << 8))))
[ This should serve the perpose required as stated in the comment above
the macro definitions. ]
5.2CursesFix2
A call from "mvcur()" to "_loc_down()" can generate incorrect cursor motion
under the following circumstances:
1. ONLCR is set in the user's terminal modes,
2. "cursor_down=^J" in the terminfo information, and
3. "cursor_down" is the "optimal" move chosen by "mvcur()"
(easily accomplished with a simple terminfo entry with NO absolute
cursor addressing).
If "mvcur()" is called and it is desired to move the cursor such that
"_loc_down(2, 3, 1, 0)" is called (I'm sure other situations can be generated
so restriction #3 won't be needed, but that's beside the point.),
"cursor_down", namely '^J' is outputted. Since ONLCR is set, a newline and
form feed are echoed just as the tty driver should do. "_loc_down()" properly
figures out that we are not at the beginning of the line (i.e. we are now at
column number 0). That's fine, but "mvcur()" doesn't know the column number
has been changed! More specifically, "oldcol" in "mvcur()" is still 3.
So in the "local motions are cheapest" section near the end of "mvcur()",
"_loc_left()" is called, and the cursor gets moved back a few spaces in its
attempt to get the cursor to column #0 (but this ends up walking back a few
columns on the end of the previous line on my terminal since my terminal has
"auto_left_margin" (i.e. bw)). This extra backing up of the cursor is caused
by the intelligent cursor movement by "_loc_down()" not being communicated
back up to "mvcur()".
There seem to be two plausible solutions of "mvcur()" not knowing about
"_loc_down()"'s possible changing of the cursor's actual column number, namely:
1. Let "mvcur()" know about it. [ This is the route I chose. ]
Since "_loc_down()" already has the logic, we might as well
use it. Have "mvcur()" pass the address of "oldcol" as
another argument to "_loc_down()", and have "_loc_down()"
change it when the situation occurs. [ Now the call to
"_loc_left()" will immediately return seeing that we are
already on the desired column, namely column #0. ]
[ Sorry, but the sources are not on this system, so I can't
include any diffs here. ]
2. Don't allow "_loc_down()" to make any changes that would affect
the cursor's column number.
Drop "col" as an argument to "_loc_down()", its references
in "_loc_down()", and the argument on all "_loc_down()" calls.
5.2CursesFix3
If OCRNL is set, moving the cursor through absolute cursor motion with a
terminal having cursor addressing such that a '\r' is required in part of the
outputted cursor motion sequence (to line #13 zero relative), the tty driver
changes (as it should) the '\r' to a '\n' and the cursor get moved to line
#10 instead of line #13!
Solutions:
1. Turn off OCRNL in "newterm()" (screen/newterm.c) and in
"m_newterm()" (screen/miniinit.c) for the mini curses users.
[ This is the quick and simple fix I used. EASIER to make sure it
works! ]
2. Add '\r' recognition and execution of the appropriate actions
IF OCRNL is on. This recognition would probably go in switch
in "tparm()" (screen/tparm.c) containing the case for a '\n' and
a comment about scratching one's head. They may want to do some
more scratching. This seems to be the BETTER solution, but it would
take some time to do (typical of "head scratching" areas of code).
I may get to it some day, but I seem to be busy enough lately.
The fixes I implemented follow for the following two files:
a) "newterm()" (screen/newterm.c): in the "#ifdef USG" where ECHO is
turned off, add:
(cur_term->Nttyb).c_oflag &= ~OCRNL;
b) "m_newterm()" (screen/miniinit.c): just after the call to "_newtty()"
and before the call to "signal(SIGTSTP, m_tstp)" (the latter is
appropriately "ifdef"ed to SIGTSTP, of course) add:
#ifdef USG
(cur_term->Nttyb).c_oflag &= ~OCRNL;
reset_prog_mode();
#endif USG
5.2CursesFix4
The "FILE" type variable "outf" is used throughout the curses library for
debugging output. There are several causes where it is assumed to NOT be zero;
i.e. the value is not checked before using it. It is possible that "outf" is
zero, and since the library is very careful to "always" check it, I feel the
following are oversights that should be fixed.
The following routines do not properly check "outf" before using it:
1. "draino()" (screen/draino.c) should have an "if (outf)" just before
the "fprintf()" that is approximately in the middle of the for loop.
[ Sorry, I don't have the sources on this system so I can't give you
any diffs. ]
2. "mvcur()" (screen/mvcur.c) should have an "if (outf)" just before
the first "fprintf()" call in "_loc_right()".
3. "_ll_refresh()" (screen/ll_refresh.c) should have an "if (outf)" just
before each "putc('\'', outf);" around line #118 and #142. I made
the following sample change, since it is the simplest and to
minimize the changes to the source (shorter diffs!).
for (j=0; j<SP->std_body[i]->length; j++)
{
n = SP->std_body[i]->body[j];
if (n & A_ATTRIBUTES)
{
if (outf) /* Add this line <------------- */
putc('\'', outf);
}
:
:
}
5.2CursesFix5
User calls to "mvscanw()" and/or "mvwscanw()" cause "__sscans()"to be undefined
when the program is being loaded (at "compile" time). In other words, the
C routine "_sscans()" was not loaded. This is because _sscans() doesn't
exist, but "__sscans()" does exist. (Remember this very problem with the
BSD curses (it was in 4.1BSD or 4.2BSD)?)
The simpliest solution is to change the call in "mvscanw()" (screen/mvscanw.c)
and in "mvwscanw()" (screen/mvwscanw.c) from "_sscans()" to "__sscans()".
5.2CursesFix6
Scrolling a window with the current position of the window's cursor on the the
top line of that window puts the cursor at an "invalid" value. This is more a
change in the code to better handle an "undefined" action than anything else.
This just might help protect the user from himself/herself.
In "_tscroll()" (screen/_tscroll.c) change:
win->_cury--;
to
if(win->_cury-- <= 0)
win->_cury = 0;
This is similar to the one I posted for 4.2 BSD curses.
5.2CursesFix7
After posting my ~6 bugs for 5.2 curses, I received the following bug
notification. I thought I would share it with the net. The wording
is mine, but FULL credit for the fix goes to "akgua!whuxle!mp (Mark Plotnick)".
Thanks Mark!
On approximately line #310 in "tparm()" (screen/tparm.c) is the following line
of code that handles the "%=" stack manipulation as documented at the top of
page 8 in the 5.2 Programmer Reference Manual concerning the parameterized
string manipulation ( inhale ). It is supposed to COMPARE, but it ASSIGNS
instead. ("%=" means to COMPARE for equality the top two stack elements (which
are popped) and push the result.)
case '=': c=pop(); op=pop(); push(op = c); break;
^
|
This is effectively putting the
top element back on top of the stack.
=> This operation only removes the
second element.
This line should be:
case '=': c=pop(); op=pop(); push(op == c); break;
5.2CursesFix8
There is a bug with the "sgr" parameter values as documented on page 10
(fourth paragraph in the "Highlighting, Underlining and Visible Bells" section)
of the terminfo(4) document in the System V.2 Programmer Reference Manual.
The paragraph talks about the "sgr"'s nine parameters. It states:
"Each parameter is either 0 or 1, as the corresponding
attribute is on or off."
Wrong! It is 0 or NONZERO. I wrote a string to describe the ability (of the
terminals which we use) to use this by multiplying the given attribute's
parameter value to shift the bit to the appropriate position and "or"ing each
value together as I processed each attribute that I could implement.
I ended up generating a character that was unchanged from my bias character
(with which I started "or"ing) since the values were not only not 1, but were
values that wouldn't even fit in a byte. This is caused by the call to
"tparm()" in screen/vidputs.c in the function "vidputs()". On approximately
line #23, you see lines like:
:
:
newmode & A_STANDOUT,
:
:
These are the values that get passed as the "parameters" for the "sgr"
attribute processing. A quick glance in curses.h will show that "A_STANDOUT"
is not 1 (nor are any of the other 8 attribute mode masks).
The fix is simple. Change each line to look like:
:
:
(newmode & A_STANDOUT) != 0,
:
:
Make sure you change all 9 attribute parameter arguments to boolean
comparisons!
Sorry, but I don't have any sources on this system so I don't have a nice
"diff" output.
Mike Laman, NCR Rancho Bernardo
UUCP: {ucbvax,philabs,sdcsla}!sdcsvax!ncr-sd!laman