top: eliminate a potential abend when exiting 'A' mode <=== port of newlib 80e2a7a6

______________________________ original newlib message
----------------------------------- ( minus git hash )

In that issue cited below, Tyson Nottingham identified
a potential abend which was associated with 'alternate
display mode' plus that troublesome 'mkVIZrow1' macro.
He also offered a perfectly adequate fix for that bug.

I refer to that macro as troublesome since it's now so
widely used and sometimes (by design) causes 'begtask'
to go negative (invalid). And now I found yet one more
place where it should have been used but wasn't ('f').

It's also troublesome as evidenced by some git history
listed below. Heck, there was even a commit addressing
the same symptoms (alternate display mode abend) which
Tyson suffered. Clearly, the current design is flawed.

So, with those two issues in mind, I've refactored the
approach to maintaining a visible task in the 1st row.
Henceforth, a 'mkVIZrow1' macro will be issued in only
two places: once at startup and after most keystrokes.

Such an approach likely results in additional calls to
the 'window_hlp' routine that aren't really necessary.
But, it provides a cleaner design less prone to errors
in the future. Besides, such additional overhead would
only be incurred when interacting with the user. Thus,
new costs are of no concern and will never be noticed.

Reference(s):
. Tyson Nottingham reported problem
https://gitlab.com/procps-ng/procps/-/issues/245
. Jun, 2018 - visible row 1 tasks first addressed
commit ........................................
. Jun, 2018 - adressed edge case, new bugs created
commit ........................................
. Sep, 2018 - additional edge case addressed
commit ........................................
. May, 2021 - some abends fixed, new error created
commit ........................................
. Jun, 2021 - try to prorect against future errors
commit ........................................
. Sep, 2021 - integrate mkVIZ & 'focused' tasks
commit ........................................

Discovered by: Tyson Nottingham
Signed-off-by: Jim Warner <james.warner@comcast.net>

diff --git a/top/top.c b/top/top.c
index 7d082e97f477e303a6358545cc6a60ca01e933c2..8ae05215c42a9c2618f1f3363346214d8b25b94d 100644 (file)
--- a/top/top.c
+++ b/top/top.c
@@ -4630,10 +4630,11 @@ static void win_names (WIN_t *q, const char *name) {
 static void win_reset (WIN_t *q) {
          SETw(q, Show_IDLEPS | Show_TASKON);
 #ifndef SCROLLVAR_NO
-         q->rc.maxtasks = q->usrseltyp = q->begpflg = q->begtask = q->begnext = q->varcolbeg = q->focus_pid = 0;
+         q->rc.maxtasks = q->usrseltyp = q->begpflg = q->begtask = q->varcolbeg = q->focus_pid = 0;
 #else
-         q->rc.maxtasks = q->usrseltyp = q->begpflg = q->begtask = q->begnext = q->focus_pid = 0;
+         q->rc.maxtasks = q->usrseltyp = q->begpflg = q->begtask = q->focus_pid = 0;
 #endif
+         mkVIZoff(q)
          osel_clear(q);
          q->findstr[0] = '\0';
          q->rc.combine_cpus = 0;
@@ -5147,7 +5148,7 @@ static void forest_excluded (WIN_t *q) {
       // watch out for newly forked/cloned tasks 'above' us ...
       if (q->begtask < q->focus_beg) {
          q->begtask = q->focus_beg;
-         q->begnext = 0;     // as 'mkVIZoff' but in any window
+         mkVIZoff(q)
       }
 #ifdef FOCUS_HARD_Y
       // if some task 'above' us ended, try to maintain focus
@@ -5155,7 +5156,7 @@ static void forest_excluded (WIN_t *q) {
       if (q->begtask > q->focus_beg
       && (SCREEN_ROWS > (q->focus_end - q->focus_beg))) {
          q->begtask = q->focus_beg;
-         q->begnext = 0;     // as 'mkVIZoff' but in any window
+         mkVIZoff(q)
       }
 #endif
    }
@@ -5626,7 +5627,6 @@ static void keys_global (int ch) {
       case '?':
       case 'h':
          help_view();
-         mkVIZrow1
          break;
       case 'B':
          TOGw(w, View_NOBOLD);
@@ -5653,7 +5653,6 @@ static void keys_global (int ch) {
          break;
       case 'g':
          win_select(0);
-         mkVIZrow1
          break;
       case 'H':
          Thread_mode = !Thread_mode;
@@ -5738,7 +5737,6 @@ static void keys_global (int ch) {
          break;
       case 'Z':
          wins_colors();
-         mkVIZrow1
          break;
       case '0':
          Rc.zero_suppress = !Rc.zero_suppress;
@@ -5991,10 +5989,7 @@ static void keys_task (int ch) {
       case 'O':
       case 'o':
       case kbd_CtrlO:
-         if (VIZCHKw(w)) {
-            other_filters(ch);
-            mkVIZrow1
-         }
+         if (VIZCHKw(w)) other_filters(ch);
          break;
       case 'U':
       case 'u':
@@ -6003,7 +5998,6 @@ static void keys_task (int ch) {
             if (*str != kbd_ESC
             && (errmsg = user_certify(w, str, ch)))
                 show_msg(errmsg);
-            mkVIZrow1
          }
          break;
       case 'V':
@@ -6109,10 +6103,7 @@ static void keys_window (int ch) {
          break;
       case 'a':
       case 'w':
-         if (ALTCHKw) {
-            win_select(ch);
-            mkVIZrow1
-         }
+         if (ALTCHKw) win_select(ch);
          break;
       case 'G':
          if (ALTCHKw) {
@@ -6209,10 +6200,8 @@ static void keys_window (int ch) {
       case kbd_HOME:
 #ifndef SCROLLVAR_NO
          if (VIZCHKw(w)) if (CHKw(w, Show_IDLEPS)) w->begtask = w->begpflg = w->varcolbeg = 0;
-         mkVIZrow1
 #else
          if (VIZCHKw(w)) if (CHKw(w, Show_IDLEPS)) w->begtask = w->begpflg = 0;
-         mkVIZrow1
 #endif
          break;
       case kbd_END:
@@ -6634,6 +6623,13 @@ static void do_key (int ch) {
                key_tab[i].func(ch);
                if (Frames_signal == BREAK_off)
                   Frames_signal = BREAK_kbd;
+               /* due to the proliferation of the need for 'mkVIZrow1', |
+                  aside from 'wins_stage_2' use, we'll now issue it one |
+                  time here. there will remain several places where the |
+                  companion 'mkVIZrowX' macro is issued, thus the check |
+                  for a value already in 'begnext' in this conditional. | */
+               if (CHKw(Curwin, Show_TASKON) && !mkVIZyes)
+                  mkVIZrow1
                goto all_done;
             }
    };
@@ -7003,14 +6999,11 @@ static const char *task_show (const WIN_t *q, const int idx) {
 
 
         /*
-         * A window_show *Helper* function ensuring that Curwin's 'begtask'
-         * represents a visible process (not any hidden/filtered-out task).
-         * In reality, this function is called:
-         *   1) exclusively for the 'current' window
-         *   2) immediately after interacting with the user
-         *   3) who struck: up, down, pgup, pgdn, home, end, 'o/O' or 'u/U'
-         *   4) or upon the user switching from one window to another window
-         * ( note: it's entirely possible there are NO visible tasks to show ) */
+         * A window_show *Helper* function ensuring that a window 'begtask' |
+         * represents a visible process (not any hidden/filtered-out task). |
+         * In reality this function is called exclusively for the 'current' |
+         * window and only after available user keystroke(s) are processed. |
+         * Note: it's entirely possible there are NO visible tasks to show! | */
 static void window_hlp (void) {
    WIN_t *w = Curwin;             // avoid gcc bloat with a local copy
    int i, reversed;
@@ -7058,7 +7051,7 @@ fwd_redux:
    }
 
 wrap_up:
-   w->begnext = 0;
+   mkVIZoff(w)
    OFFw(w, NOPRINT_xxx);
 } // end: window_hlp
 

diff --git a/top/top.h b/top/top.h
index 70ba33d7c424135b6a54512ae47eff66e26043dd..ea8420eed39df685f5ef6960bcc83adc2be70389 100644 (file)
--- a/top/top.h
+++ b/top/top.h
@@ -509,9 +509,10 @@ typedef struct WIN_t {
 
         // Support for a proper (visible) row #1 whenever Curwin changes
         // ( or a key which might affect vertical scrolling was struck )
-#define mkVIZyes       Curwin->begnext != 0
+#define mkVIZyes     ( Curwin->begnext != 0 )
 #define mkVIZrow1    { Curwin->begnext = +1; Curwin->begtask -= 1; }
 #define mkVIZrowX(n) { Curwin->begnext = (n); }
+#define mkVIZoff(w)  { w->begnext = 0; }
 
         /* Special Section: end ------------------------------------------ */
         /* /////////////////////////////////////////////////////////////// */