top: protect against the anomalous 'Mem' graph display

Until this patch, top falsely assumed that there would
always be some (small) amount of physical memory after
subtracting 'used' and 'available' from the total. But
as the issue referenced below attests, a sum of 'used'
and 'available' might exceed that total memory amount.

I'm not sure if this is a problem with our calculation
of the 'used' amount, a flaw in the kernel 'available'
algorithms or some other reason I cannot even imagine.

Anyway, this patch protects against such a contingency
through the following single line addition of new code
. if (pct_used + pct_misc > 100.0 || pct_misc < 0) ...

The check for less than zero is not actually necessary
as long as the source numbers remain unsigned. However
should they ever become signed, we'll have protection.

[ Most of the changes in this commit simply separate ]
[ a variable's definition from its associated logic. ]

Reference(s):
https://gitlab.com/procps-ng/procps/issues/64

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

diff --git a/NEWS b/NEWS
index fa0e23aed5924c4de1818bba0ecd48f2dc3b46c7..f5ef2adc01c2aae499ff99163975ffc4066c64d8 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -20,6 +20,7 @@ procps-ng-NEXT
   * top: fix argument parsing quirk resulting in SEGV      Redhat #1450429
   * top: delay interval accepts non-locale radix point     Redhat #1182248
   * top: address a wishlist man page NLS suggestion        Debian #865689
+  * top: fix potential distortion in 'Mem' graph display   issue #64
   * watch: define HOST_NAME_MAX where not defined          Debian #830734
 
 procps-ng-3.3.12

diff --git a/top/top.c b/top/top.c
index 385df1d564743f426ee060b1f54206f127a53597..948805e468d67293163d1f8cd4f9dada66d75464 100644 (file)
--- a/top/top.c
+++ b/top/top.c
@@ -5249,21 +5249,26 @@ numa_nope:
             { "%-.*s~4", "%-.*s~6", "%-.*s~6", Graph_blks }
          };
          char used[SMLBUFSIZ], util[SMLBUFSIZ], dual[MEDBUFSIZ];
-         int ix = w->rc.graph_mems - 1;
-         float pct_used = (float)kb_main_used * (100.0 / (float)kb_main_total),
+         float pct_used, pct_misc, pct_swap;
+         int ix, num_used, num_misc;
+
+         pct_used = (float)kb_main_used * (100.0 / (float)kb_main_total);
 #ifdef MEMGRAPH_OLD
-               pct_misc = (float)(kb_main_buffers + kb_main_cached) * (100.0 / (float)kb_main_total),
+         pct_misc = (float)(kb_main_buffers + kb_main_cached) * (100.0 / (float)kb_main_total);
 #else
-               pct_misc = (float)(kb_main_total - kb_main_available - kb_main_used) * (100.0 / (float)kb_main_total),
+         pct_misc = (float)(kb_main_total - kb_main_available - kb_main_used) * (100.0 / (float)kb_main_total);
 #endif
-               pct_swap = kb_swap_total ? (float)kb_swap_used * (100.0 / (float)kb_swap_total) : 0;
+         if (pct_used + pct_misc > 100.0 || pct_misc < 0) pct_misc = 0;
+         pct_swap = kb_swap_total ? (float)kb_swap_used * (100.0 / (float)kb_swap_total) : 0;
+         ix = w->rc.graph_mems - 1;
 #ifndef QUICK_GRAPHS
-         int num_used = (int)((pct_used * Graph_adj) + .5),
-             num_misc = (int)((pct_misc * Graph_adj) + .5);
+         num_used = (int)((pct_used * Graph_adj) + .5),
+         num_misc = (int)((pct_misc * Graph_adj) + .5);
          if (num_used + num_misc > Graph_len) num_misc = Graph_len - num_used;
          snprintf(used, sizeof(used), gtab[ix].used, num_used, gtab[ix].type);
          snprintf(util, sizeof(util), gtab[ix].misc, num_misc, gtab[ix].type);
 #else
+         (void)num_used; (void)num_misc;
          snprintf(used, sizeof(used), gtab[ix].used, (int)((pct_used * Graph_adj) + .5), gtab[ix].type);
          snprintf(util, sizeof(util), gtab[ix].misc, (int)((pct_misc * Graph_adj) + .4), gtab[ix].type);
 #endif