"curses" bug fixes
Guy Harris
guy at sun.uucp
Wed Feb 26 20:08:36 AEST 1986
Some bug fixes to "curses". Some of these came from a list Mike Laman
posted, and others were discovered when I tried to test the 4.3BSD
"canfield" program with the S5 "curses". The annoying thing is that I
remember fixing two of the latter bugs when I was at CCI; I didn't take any
source with me, so I had to fix them all over again. Those who do not
remember the past are doomed to repeat it...
The "diffs" refer to a version of the S5R2 "curses" distributed as part of
the BRL compatibility package. Your line numbers will vary; these line
numbers should be used for comparison purposes only. California line
numbers may be lower.
The problems I found were:
1) When redrawing the screen after doing an "endwin" (i.e., when doing a
shell escape or a suspend), it would draw spaces with video enhancements
where they didn't belong. The problem is that the code to move the cursor
to the right would try to do so by doing a bunch of tabs and then either a
bunch of "cursor right" sequences or a reprinting of the characters
currently on the screen. If the characters on the screen didn't have the
same video attributes as the current physical screen's video attributes, it
would use the "cursor right" sequence; however, if the current line was
blank and unallocated it would assume that spaces were OK. Needless to say,
they are only OK if the current physical screen has no video attributes
active. This required a fix to "mvcur.c".
2) When doing the aforementioned redraw with a "wrefresh(curscr)", the
screen would get repainted twice. The trouble is that "wrefresh" calls
"wnoutrefresh" and "doupdate"; when the window is "curscr" "wnoutrefresh"
just calls "_ll_refresh", but "doupdate" also calls "_ll_refresh". The fix
is not to call "wnoutrefresh" if "win" is "curscr" and "curses" is being
re-entered.
3) "tic" does not return a correct exit status - it falls off the end (sigh)
and dereferences NULL pointers (double sigh).
Here's Mike's original article:
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 3Bs use unsigned characters - gh)
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.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
Diffs:
*** /arch/s5r2compat/src/libcurses/screen/_tscroll.c Wed Jan 30 19:59:51 1985
--- _tscroll.c Wed Feb 26 01:38:31 1986
***************
*** 57,63
fprintf( outf, "Doing --> touchwin( 0%o )\n", win );
}
#endif DEBUG
! win->_cury--;
/*
** This section taken out because it wasn't allowing the scrolling of a
** region smaller than the full screen. Taken out on 10/12/83. The
--- 57,64 -----
fprintf( outf, "Doing --> touchwin( 0%o )\n", win );
}
#endif DEBUG
! if (win->_cury > 0)
! win->_cury--;
/*
** This section taken out because it wasn't allowing the scrolling of a
** region smaller than the full screen. Taken out on 10/12/83. The
*** /arch/s5r2compat/src/libcurses/screen/draino.c Wed Jan 30 20:00:03 1985
--- draino.c Wed Feb 26 01:37:31 1986
***************
*** 40,46
ncthere = 0;
rv = ioctl(cur_term->Filedes, TIOCOUTQ, &ncthere);
#ifdef DEBUG
! fprintf(outf, "draino: rv %d, ncneeded %d, ncthere %d\n",
rv, ncneeded, ncthere);
#endif
if (rv < 0)
--- 40,46 -----
ncthere = 0;
rv = ioctl(cur_term->Filedes, TIOCOUTQ, &ncthere);
#ifdef DEBUG
! if(outf) fprintf(outf, "draino: rv %d, ncneeded %d, ncthere %d\n",
rv, ncneeded, ncthere);
#endif
if (rv < 0)
*** /arch/s5r2compat/src/libcurses/screen/ll_refresh.c Wed Jan 30 20:00:44 1985
--- ll_refresh.c Wed Feb 26 01:36:31 1986
***************
*** 115,121
n = SP->cur_body[i]->body[j];
if( n & A_ATTRIBUTES )
{
! putc('\'', outf);
}
n &= 0177;
if(outf) fprintf(outf, "%c", n>=' ' ? n : '.');
--- 115,121 -----
n = SP->cur_body[i]->body[j];
if( n & A_ATTRIBUTES )
{
! if(outf) putc('\'', outf);
}
n &= 0177;
if(outf) fprintf(outf, "%c", n>=' ' ? n : '.');
*** /arch/s5r2compat/src/libcurses/screen/miniinit.c Wed Jan 30 20:00:13 1985
--- miniinit.c Wed Feb 26 01:45:17 1986
***************
*** 63,68
SP->input_file = infd;
savetty();
scp = _new_tty(type, outfd);
# ifdef SIGTSTP
signal(SIGTSTP, m_tstp);
# endif
--- 63,72 -----
SP->input_file = infd;
savetty();
scp = _new_tty(type, outfd);
+ # ifdef USG
+ (cur_term->Nttyb).c_oflag &= ~OCRNL;
+ reset_prog_mode();
+ # endif USG
# ifdef SIGTSTP
signal(SIGTSTP, m_tstp);
# endif
*** /arch/s5r2compat/src/libcurses/screen/mvcur.c Wed Jan 30 20:00:15 1985
--- mvcur.c Wed Feb 26 01:35:41 1986
***************
*** 236,242
if (newcol == oldcol)
return 0; /* already there - nothing to do */
#ifdef DEBUG
! fprintf(outf, "SP %x, phys_irm %d, notinsmode %d\n",
SP, SP->phys_irm, notinsmode);
#endif
--- 236,242 -----
if (newcol == oldcol)
return 0; /* already there - nothing to do */
#ifdef DEBUG
! if (outf) fprintf(outf, "SP %x, phys_irm %d, notinsmode %d\n",
SP, SP->phys_irm, notinsmode);
#endif
***************
*** 318,324
row, tabcol, i, tabcol+i, SP->cur_body[row+1]->body[tabcol+i]);
#endif
rp = SP->cur_body[row+1];
! if (cursor_right && (!notinsmode || rp && SP->phys_gr != (rp->body[tabcol+i] & A_ATTRIBUTES))) /* dont know */
tputs(cursor_right,1,_outch);
else if (rp && rp->length > tabcol+i)
/* Note we assume dumb terminals without cursor_right don't have
--- 318,327 -----
row, tabcol, i, tabcol+i, SP->cur_body[row+1]->body[tabcol+i]);
#endif
rp = SP->cur_body[row+1];
! if (cursor_right &&
! (!notinsmode ||
! rp ? SP->phys_gr != (rp->body[tabcol+i] & A_ATTRIBUTES) :
! SP->phys_gr != 0)) /* dont know */
tputs(cursor_right,1,_outch);
else if (rp && rp->length > tabcol+i)
/* Note we assume dumb terminals without cursor_right don't have
*** /arch/s5r2compat/src/libcurses/screen/mvscanw.c Wed Jan 30 20:00:16 1985
--- mvscanw.c Wed Feb 26 01:34:02 1986
***************
*** 14,18
char *fmt;
int args; {
! return move(y, x) == OK ? _sscans(stdscr, fmt, &args) : ERR;
}
--- 14,18 -----
char *fmt;
int args; {
! return move(y, x) == OK ? __sscans(stdscr, fmt, &args) : ERR;
}
*** /arch/s5r2compat/src/libcurses/screen/mvwscanw.c Wed Jan 30 20:00:17 1985
--- mvwscanw.c Wed Feb 26 01:34:40 1986
***************
*** 8,12
char *fmt;
int args; {
! return wmove(win, y, x) == OK ? _sscans(win, fmt, &args) : ERR;
}
--- 8,12 -----
char *fmt;
int args; {
! return wmove(win, y, x) == OK ? __sscans(win, fmt, &args) : ERR;
}
*** /arch/s5r2compat/src/libcurses/screen/newterm.c Wed Jan 30 20:00:18 1985
--- ./newterm.c Wed Feb 26 01:58:43 1986
***************
*** 37,42
return NULL;
#ifdef USG
(cur_term->Nttyb).c_lflag &= ~ECHO;
#else
(cur_term->Nttyb).sg_flags &= ~ECHO;
#endif
--- 37,43 -----
return NULL;
#ifdef USG
(cur_term->Nttyb).c_lflag &= ~ECHO;
+ (cur_term->Nttyb).c_oflag &= ~OCRNL;
#else
(cur_term->Nttyb).sg_flags &= ~ECHO;
#endif
*** /arch/s5r2compat/src/libcurses/screen/setupterm.c Wed Jan 30 20:00:27 1985
--- setupterm.c Wed Feb 26 01:42:01 1986
***************
*** 33,38
#ifdef pdp11
#define getsh(ip) (* (short *) ip)
#endif
#ifndef getsh
/*
--- 33,41 -----
#ifdef pdp11
#define getsh(ip) (* (short *) ip)
#endif
+ #ifdef BIG_ENDIAN_MACHINE
+ #define getsh(ip) ((short) (*((unsigned char *) ip) | (*(ip+1) << 8)))
+ #endif
#ifndef getsh
/*
***************
*** 43,49
register char *p;
{
register int rv;
! if (*p == 0377)
return -1;
rv = *p++;
rv += *p * 256;
--- 46,54 -----
register char *p;
{
register int rv;
! rv = *((unsigned char *)(p++));
! rv += *((unsigned char *)p) << 8;
! if (rv == 0xffff)
return -1;
return rv;
}
***************
*** 45,52
register int rv;
if (*p == 0377)
return -1;
- rv = *p++;
- rv += *p * 256;
return rv;
}
#endif
--- 50,55 -----
rv += *((unsigned char *)p) << 8;
if (rv == 0xffff)
return -1;
return rv;
}
#endif
*** /arch/s5r2compat/src/libcurses/screen/tic.c Wed Jan 30 20:00:35 1985
--- tic.c Wed Feb 26 01:33:14 1986
***************
*** 86,91
else for (i=1; i<argc; i++) {
compfile(fopen(argv[i], "r"), argv[i]);
}
}
/*
--- 86,92 -----
else for (i=1; i<argc; i++) {
compfile(fopen(argv[i], "r"), argv[i]);
}
+ exit(0);
}
/*
***************
*** 114,120
for (;;) {
if (fgets(ibuf, sizeof ibuf, tf) == NULL) {
fclose(tf);
! if (tnchkuse(fname))
store(bp);
return 0;
}
--- 115,121 -----
for (;;) {
if (fgets(ibuf, sizeof ibuf, tf) == NULL) {
fclose(tf);
! if (*bp && tnchkuse(fname))
store(bp);
return 0;
}
*** /arch/s5r2compat/src/libcurses/screen/tparm.c Wed Jan 30 20:00:36 1985
--- tparm.c Wed Feb 26 01:31:46 1986
***************
*** 307,313
case '&': c=pop(); op=pop(); push(op & c); break;
case '|': c=pop(); op=pop(); push(op | c); break;
case '^': c=pop(); op=pop(); push(op ^ c); break;
! case '=': c=pop(); op=pop(); push(op = c); break;
case '>': c=pop(); op=pop(); push(op > c); break;
case '<': c=pop(); op=pop(); push(op < c); break;
--- 307,313 -----
case '&': c=pop(); op=pop(); push(op & c); break;
case '|': c=pop(); op=pop(); push(op | c); break;
case '^': c=pop(); op=pop(); push(op ^ c); break;
! case '=': c=pop(); op=pop(); push(op == c); break;
case '>': c=pop(); op=pop(); push(op > c); break;
case '<': c=pop(); op=pop(); push(op < c); break;
*** /arch/s5r2compat/src/libcurses/screen/vidputs.c Wed Jan 30 20:00:39 1985
--- vidputs.c Wed Feb 26 01:30:23 1986
***************
*** 21,35
if (newmode || !exit_attribute_mode) {
if (set_attributes) {
tputs(tparm(set_attributes,
! newmode & A_STANDOUT,
! newmode & A_UNDERLINE,
! newmode & A_REVERSE,
! newmode & A_BLINK,
! newmode & A_DIM,
! newmode & A_BOLD,
! newmode & A_INVIS,
! newmode & A_PROTECT,
! newmode & A_ALTCHARSET),
1, outc);
curmode = newmode;
} else {
--- 21,35 -----
if (newmode || !exit_attribute_mode) {
if (set_attributes) {
tputs(tparm(set_attributes,
! (newmode & A_STANDOUT) != 0,
! (newmode & A_UNDERLINE) != 0,
! (newmode & A_REVERSE) != 0,
! (newmode & A_BLINK) != 0,
! (newmode & A_DIM) != 0,
! (newmode & A_BOLD) != 0,
! (newmode & A_INVIS) != 0,
! (newmode & A_PROTECT) != 0,
! (newmode & A_ALTCHARSET) != 0),
1, outc);
curmode = newmode;
} else {
*** /arch/s5r2compat/src/libcurses/screen/wrefresh.c Wed Jan 30 20:00:42 1985
--- wrefresh.c Wed Feb 26 01:29:02 1986
***************
*** 12,17
wrefresh(win)
WINDOW *win;
{
! wnoutrefresh(win);
return doupdate();
}
--- 12,30 -----
wrefresh(win)
WINDOW *win;
{
! extern int _endwin;
!
! /*
! * If "win" is "curscr", the call to "wnoutrefresh" will just result
! * in a call to "_ll_refresh"; the call to "doupdate" will then
! * result in another one. The first call will say "don't use
! * insert/delete line", the second call will use it if "idlok"
! * has been called on "curscr". If "_endwin" is true. the screen
! * will get cleared before the second call anyway, so the value of
! * the "insert/delete OK" flag is irrelevant; don't paint the
! * screen twice.
! */
! if (win != curscr || !_endwin)
! wnoutrefresh(win);
return doupdate();
}
--
Guy Harris
{ihnp4, decvax, seismo, decwrl, ...}!sun!guy
guy at sun.arpa (yes, really)
More information about the Net.bugs.usg
mailing list