]> granicus.if.org Git - procps-ng/commitdiff
top: avoid library shame with refactored 'Ctrl' window
authorJim Warner <james.warner@comcast.net>
Wed, 4 May 2022 05:00:00 +0000 (00:00 -0500)
committerCraig Small <csmall@dropbear.xyz>
Wed, 4 May 2022 06:45:57 +0000 (16:45 +1000)
Well darn it, whoever wrote that new library caught me
with my pants down (again?). Shoot, they were not just
down but somehow missing altogether. Here's the story.

Any item from that library supported by dynamic memory
can only be represented in user's stacks exactly once.

Should any string based enumerator be duplicated among
the items array, for any instance beyond the first the
library will return '[ duplicate ENUM ]' for a result.

That's where I lost my pants. While command lines were
given special handling (and never duplicated) I failed
to turn on CGROUPS, SUPGRPS & ENVIRON when testing the
Ctrl-G, Ctrl-U & Ctrl-N keys. If any of those 3 are on
that's when a Ctrl window sees a 'duplicate' notation.

[ and who runs top with such fields displayed anyway ]

In responding to this oops, the internals were changed
quite dramatically & vastly simplified in the process.

More importantly, the 'duplicate' results are no more.

Signed-off-by: Jim Warner <james.warner@comcast.net>
top/top.c
top/top.h

index d475aaa483a932e071c86d36bc2eff73f9d1c8a9..2bc7d45e67ff220691c0f37cd96e850dbb09a108 100644 (file)
--- a/top/top.c
+++ b/top/top.c
@@ -106,15 +106,14 @@ static int   Screen_cols, Screen_rows, Max_lines;
 #define      SCREEN_ROWS ( Screen_rows - Tagged_rsvd )
         // 1 for horizontal separator
 #define      TAGGED_RSVD ( 1 )
-#define      TAGGED_KEEP Tagged_func = NULL
-#define      TAGGED_TOSS do { Tagged_func = NULL; \
-                Tagged_task = Tagged_rsvd = Tagged_enum = 0; \
-                Fieldstab[eu_GENERIC].item = PIDS_extra; } while(0)
-static int   Tagged_task,
+#define      TAGGED_KEEP Tagged_show = 0
+#define      TAGGED_TOSS do { Tagged_show = 0; \
+                Tagged_task = Tagged_rsvd = Tagged_enum = 0; } while(0)
+static int   Tagged_show,
              Tagged_rsvd,
-             Tagged_enum;
+             Tagged_task;
+enum pflag   Tagged_enum;
 static char *Tagged_name;
-static void(*Tagged_func)(void);
 
         /* This is really the number of lines needed to display the summary
            information (0 - nn), but is used as the relative row where we
@@ -1902,14 +1901,12 @@ static struct {
 #define eu_TREE_HID    eu_LAST +4
 #define eu_TREE_LVL    eu_LAST +5
 #define eu_TREE_ADD    eu_LAST +6
-#define eu_GENERIC     eu_LAST +7
    , {  -1, -1, -1,  PIDS_CMDLINE     }  // str      ( if Show_CMDLIN, eu_CMDLINE    )
    , {  -1, -1, -1,  PIDS_TICS_ALL_C  }  // ull_int  ( if Show_CTIMES, eu_TICS_ALL_C )
    , {  -1, -1, -1,  PIDS_ID_FUID     }  // u_int    ( if a usrseltyp, eu_ID_FUID    )
    , {  -1, -1, -1,  PIDS_extra       }  // s_ch     ( if Show_FOREST, eu_TREE_HID   )
    , {  -1, -1, -1,  PIDS_extra       }  // s_int    ( if Show_FOREST, eu_TREE_LVL   )
    , {  -1, -1, -1,  PIDS_extra       }  // s_int    ( if Show_FOREST, eu_TREE_ADD   )
-   , {  -1, -1, -1,  PIDS_extra       }  // str      {  special 'tag', eu_GENERIC    }
  #undef A_left
  #undef A_right
 };
@@ -4630,9 +4627,10 @@ static void wins_stage_2 (void) {
 
 
         /*
-         * This guy manages the bottom margin window |
-         * & the tagged process command line display | */
-static void wins_tag_cmdline (void) {
+         * This guy manages the bottom margin window,
+         * showing miscellaneous variable width data.  */
+static void wins_tag_show (void) {
+ #define maxRSVD  ( Screen_rows - 1 )
    char buf[SMLBUFSIZ];
    const char *p;
    int i;
@@ -4642,11 +4640,12 @@ static void wins_tag_cmdline (void) {
          break;
    }
    if (i < PIDSmaxt) {
-      snprintf(buf, sizeof(buf), " command line for pid %d, %s"
-         , Tagged_task, PID_VAL(EU_CMD, str, Curwin->ppt[i]));
-      p = PID_VAL(eu_CMDLINE, str, Curwin->ppt[i]);
-      if (!p || !*p) p = "n/a";
+      snprintf(buf, sizeof(buf), " %s for pid %d, %s"
+         , Tagged_name, Tagged_task, PID_VAL(EU_CMD, str, Curwin->ppt[i]));
+      p = PID_VAL(Tagged_enum, str, Curwin->ppt[i]);
+      if (!p || !*p || !strcmp(p, "-")) p = "n/a";
       Tagged_rsvd = 1 + TAGGED_RSVD + (strlen(p) / Screen_cols);
+      if (Tagged_rsvd > maxRSVD) Tagged_rsvd = maxRSVD;
       putp(fmtmk("%s%s%-*s", tg2(0, SCREEN_ROWS), Curwin->capclr_hdr, Screen_cols, buf));
       putp(fmtmk("%s%s", tg2(0, SCREEN_ROWS + 1), Cap_clr_eos));
       putp(fmtmk("%s%s", tg2(0, SCREEN_ROWS + 1), Cap_norm));
@@ -4658,39 +4657,26 @@ static void wins_tag_cmdline (void) {
    }
    TAGGED_KEEP;
 #endif
-} // end: wins_tag_cmdline
+ #undef maxRSVD
+} // end: wins_tag_show
 
 
         /*
-         * This guy manages the bottom margin window |
-         * showing miscellaneous variable width data | */
-static void wins_tag_generic (void) {
-   char buf[SMLBUFSIZ];
-   const char *p;
-   int i;
+         * This guy toggles between displaying a Ctrl
+         * bottom window or arranging to turn it off. */
+static void wins_tag_toggle (enum pflag enu, const char *str) {
+   int pid = PID_VAL(EU_PID, s_int, Curwin->ppt[Curwin->begtask]);
 
-   for (i = 0; i < PIDSmaxt; i++) {
-      if (Tagged_task == PID_VAL(EU_PID, s_int, Curwin->ppt[i]))
-         break;
-   }
-   if (i < PIDSmaxt) {
-      snprintf(buf, sizeof(buf), " %s for pid %d, %s"
-         , Tagged_name, Tagged_task, PID_VAL(EU_CMD, str, Curwin->ppt[i]));
-      p = PID_VAL(eu_GENERIC, str, Curwin->ppt[i]);
-      if (!p || !*p || !strcmp(p, "-")) p = "n/a";
-      Tagged_rsvd = 1 + TAGGED_RSVD + (strlen(p) / Screen_cols);
-      putp(fmtmk("%s%s%-*s", tg2(0, SCREEN_ROWS), Curwin->capclr_hdr, Screen_cols, buf));
-      putp(fmtmk("%s%s", tg2(0, SCREEN_ROWS + 1), Cap_clr_eos));
-      putp(fmtmk("%s%s", tg2(0, SCREEN_ROWS + 1), Cap_norm));
-      fputs(p, stdout);
-#ifdef TAGGED_BRIEF
-   } else
+   // if already targeted, assume user wants to turn it off ...
+   if (Tagged_enum == enu) {
       TAGGED_TOSS;
-#else
+   } else {
+      Tagged_task = pid;
+      Tagged_enum = enu;
+      Tagged_name = (char*)str;
+      Tagged_show = 1;
    }
-   TAGGED_KEEP;
-#endif
-} // end: wins_tag_generic
+} // end: wins_tag_toggle
 
 
         /*
@@ -5317,42 +5303,13 @@ static void keys_global (int ch) {
 #endif
          break;
       case kbd_CtrlG:
-         def = PID_VAL(EU_PID, s_int, w->ppt[w->begtask]);
-         // if already targeted, assume user wants to turn it off ...
-         if (Tagged_task && Fieldstab[eu_GENERIC].item == PIDS_CGROUP) {
-            TAGGED_TOSS;
-         } else {
-            Tagged_task = def;
-            Tagged_enum = eu_GENERIC;
-            Tagged_name = "control groups";
-            Tagged_func = wins_tag_generic;
-            Fieldstab[eu_GENERIC].item = PIDS_CGROUP;
-         }
+         wins_tag_toggle(EU_CGR, "control groups");
          break;
       case kbd_CtrlK:
-         def = PID_VAL(EU_PID, s_int, w->ppt[w->begtask]);
-         // if already targeted, assume user wants to turn it off ...
-         if (Tagged_task && Tagged_enum == eu_CMDLINE) {
-            TAGGED_TOSS;
-         } else {
-            Tagged_task = def;
-            Tagged_enum = eu_CMDLINE;
-            Tagged_func = wins_tag_cmdline;
-            Fieldstab[eu_GENERIC].item = PIDS_extra;
-         }
+         wins_tag_toggle(eu_CMDLINE, "command line");
          break;
       case kbd_CtrlN:
-         def = PID_VAL(EU_PID, s_int, w->ppt[w->begtask]);
-         // if already targeted, assume user wants to turn it off ...
-         if (Tagged_task && Fieldstab[eu_GENERIC].item == PIDS_ENVIRON) {
-            TAGGED_TOSS;
-         } else {
-            Tagged_task = def;
-            Tagged_enum = eu_GENERIC;
-            Tagged_name = "environment";
-            Tagged_func = wins_tag_generic;
-            Fieldstab[eu_GENERIC].item = PIDS_ENVIRON;
-         }
+         wins_tag_toggle(EU_ENV, "environment");
          break;
       case kbd_CtrlR:
          if (Secure_mode)
@@ -5381,17 +5338,7 @@ static void keys_global (int ch) {
          }
          break;
       case kbd_CtrlU:
-         def = PID_VAL(EU_PID, s_int, w->ppt[w->begtask]);
-         // if already targeted, assume user wants to turn it off ...
-         if (Tagged_task && Fieldstab[eu_GENERIC].item == PIDS_SUPGROUPS) {
-            TAGGED_TOSS;
-         } else {
-            Tagged_task = def;
-            Tagged_enum = eu_GENERIC;
-            Tagged_name = "supplementary groups";
-            Tagged_func = wins_tag_generic;
-            Fieldstab[eu_GENERIC].item = PIDS_SUPGROUPS;
-         }
+         wins_tag_toggle(EU_SGN, "supplementary groups");
          break;
       case kbd_ENTER:             // these two have the effect of waking us
       case kbd_SPACE:             // from 'pselect', refreshing the display
@@ -6895,7 +6842,7 @@ static void frame_make (void) {
    }
 
    if (CHKw(w, View_SCROLL) && VIZISw(Curwin)) show_scroll();
-   if (Tagged_func) Tagged_func();
+   if (Tagged_show) wins_tag_show();
    fflush(stdout);
 
    /* we'll deem any terminal not supporting tgoto as dumb and disable
index 49c0949f3549d0b245967bde0a51aa580e717635..21db89fe85a0853cfa97861c0159e88d0e671a00 100644 (file)
--- a/top/top.h
+++ b/top/top.h
@@ -726,8 +726,8 @@ typedef struct WIN_t {
 //atic void          wins_reflag (int what, int flg);
 //atic void          wins_stage_1 (void);
 //atic void          wins_stage_2 (void);
-//atic void          wins_tag_cmdline (void);
-//atic void          wins_tag_generic (void);
+//atic void          wins_tag_show (void);
+//atic void          wins_tag_toggle (enum pflag enu, const char *str);
 //atic inline int    wins_usrselect (const WIN_t *q, int idx);
 /*------  Forest View support  -------------------------------------------*/
 //atic void          forest_adds (const int self, int level);