guy@sun.uucp (Guy Harris) (02/26/86)
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@sun.arpa (yes, really)