From 80e2a7a682a3f351693f39120b19a6c061b023af Mon Sep 17 00:00:00 2001 From: Jim Warner Date: Tue, 26 Jul 2022 00:00:00 -0500 Subject: [PATCH] top: eliminate a potential abend when exiting 'A' mode 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 6aedeac667267b24ba7f03914eec768d661651ed . Jun, 2018 - adressed edge case, new bugs created commit 9d59ddc4661453dc65a8fc81dd75bfea40b7696c . Sep, 2018 - additional edge case addressed commit 59f02f19c796cdd83a9ec8751f83e10c42080a44 . May, 2021 - some abends fixed, new error created commit 8281ac4f98cf04c51cbeb746d214201531d660ec . Jun, 2021 - try to prorect against future errors commit 2ea082b4af6f4751548e7583fe7430cd97935318 . Sep, 2021 - integrate mkVIZ & 'focused' tasks commit 69978e365043f27305e487709474947bb377084d Discovered by: Tyson Nottingham Signed-off-by: Jim Warner --- top/top.c | 47 ++++++++++++++++++++--------------------------- top/top.h | 3 ++- 2 files changed, 22 insertions(+), 28 deletions(-) diff --git a/top/top.c b/top/top.c index ae8eb02d..5a238de3 100644 --- a/top/top.c +++ b/top/top.c @@ -4438,10 +4438,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; @@ -4910,7 +4911,7 @@ static void forest_config (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 @@ -4918,7 +4919,7 @@ static void forest_config (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 } @@ -5463,7 +5464,6 @@ static void keys_global (int ch) { case '?': case 'h': help_view(); - mkVIZrow1 break; case 'B': TOGw(w, View_NOBOLD); @@ -5490,7 +5490,6 @@ static void keys_global (int ch) { break; case 'g': win_select(0); - mkVIZrow1 break; case 'H': Thread_mode = !Thread_mode; @@ -5575,7 +5574,6 @@ static void keys_global (int ch) { break; case 'Z': wins_colors(); - mkVIZrow1 break; case '0': Rc.zero_suppress = !Rc.zero_suppress; @@ -5858,10 +5856,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': @@ -5870,7 +5865,6 @@ static void keys_task (int ch) { if (*str != kbd_ESC && (errmsg = user_certify(w, str, ch))) show_msg(errmsg); - mkVIZrow1 } break; case 'V': @@ -5976,10 +5970,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) { @@ -6076,10 +6067,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: @@ -6525,6 +6514,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; } }; @@ -6911,14 +6907,11 @@ static const char *task_show (const WIN_t *q, 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; @@ -6966,7 +6959,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 b843b511..e45d3ecd 100644 --- a/top/top.h +++ b/top/top.h @@ -440,9 +440,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 ------------------------------------------ */ /* /////////////////////////////////////////////////////////////// */