Hi David,
I'm sending an improved patch based on your comments.
Not only does it not care about the PID_MAX value,
it searches all the contents to be output to recognize the required column width
and dynamically allocates the space for PID and PPID appropriately without
creating a lot of empty space.
Additionally, this patch still allows user names to be displayed up to 8
characters without truncation.
Can you look through the attachment?
(0002-Allocate-PID-PPID-space-dynamically-in-top-command.patch)
BR-Matthew Chae
________________________________
From: David Laight <david.lai...@aculab.com>
Sent: Thursday, November 23, 2023 6:10 PM
To: 'Denys Vlasenko' <vda.li...@googlemail.com>; Matthew Chae
<matthew.c...@axis.com>
Cc: busybox@busybox.net <busybox@busybox.net>; Christopher Wong
<christopher.w...@axis.com>
Subject: RE: fix large PID overflow their column in top command
...
> + fp = xfopen_for_read("/proc/sys/kernel/pid_max");
> + if (!fgets(pid_buf, PID_DIGITS_MAX + 1, fp)) {
> ...
> + if (strncmp(pid_buf, "32768", 5) == 0)
> + pid_digits_num = 5;
> + else
> + pid_digits_num = PID_DIGITS_MAX;
>
> The logic above is not sound. Even if sysctl kernel.pid_max
> is 32768, there can be already running processes with pids > 99999.
It's also probably wrong for pretty much all other values.
I'd just base the column width on strlen(pid_buf) with a minimum
value of 5.
It is unlikely that pid_max has been reduced - so column overflow
it that case probably doesn't really matter.
The more interesting case is really a system with a very large pid_max
that has never run many processes.
You don't really want lots of blank space.
I can't remember whether top reads everything before doing any output?
Since the output is sorted it probably almost always does.
In which case it knows the column width it will need.
I did post a patch a while back that enabled 'Irix mode'.
(100% cpu is one cpu at 100%, not all cpus at 100%)
Maybe I should dig it out again.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT,
UK
Registration No: 1397386 (Wales)
From 4d24591cabe7ddd2305cbe171a30ac33f10234eb Mon Sep 17 00:00:00 2001
From: Matthew Chae <matth...@axis.com>
Date: Mon, 5 Feb 2024 09:23:59 +0100
Subject: [PATCH] Allocate PID/PPID space dynamically in top command
The presence of a large number of PID and PPID digits in the column may
cause overflow, making it impossible to display the entire data
accurately. This dynamically allocates the appropriate space for the
number of digits in the PID and PPID, ensuring the correct
representation of values in each column and guaranteeing a clean
display.
Reviewed-by: Christopher Wong <christopher.w...@axis.com>
Signed-off-by: Matthew(Sukyoung) Chae <matth...@axis.com>
---
procps/top.c | 80 +++++++++++++++++++++++++++++++++-------------------
1 file changed, 51 insertions(+), 29 deletions(-)
diff --git a/procps/top.c b/procps/top.c
index ff77542..ed4fae4 100644
--- a/procps/top.c
+++ b/procps/top.c
@@ -602,6 +602,22 @@ static unsigned long display_header(int scr_width, int *lines_rem_p)
return meminfo[MI_MEMTOTAL];
}
+static int count_digits(unsigned num)
+{
+ int cnt = 0;
+
+ if (num < 10)
+ return 1;
+
+ while (num > 0)
+ {
+ num /= 10;
+ cnt++;
+ }
+
+ return cnt;
+}
+
static NOINLINE void display_process_list(int lines_rem, int scr_width)
{
enum {
@@ -619,12 +635,40 @@ static NOINLINE void display_process_list(int lines_rem, int scr_width)
unsigned busy_jifs;
#endif
+#define PID_STR "PID"
+#define PPID_STR "PPID"
+#define DEFAULT_DIGIT_MIN 5
+
+ char top_title_str[scr_width + 1];
+ int lines = (lines_rem - 1);
+ unsigned pid_max = 0;
+ unsigned ppid_max = 0;
+ unsigned pid_digits_max = DEFAULT_DIGIT_MIN;
+ unsigned ppid_digits_max = DEFAULT_DIGIT_MIN;
+ top_status_t *s_tmp = top + G_scroll_ofs;
+
+ if (lines > ntop - G_scroll_ofs)
+ lines = ntop - G_scroll_ofs;
+
+ while (--lines >= 0) {
+ pid_max = MAX(pid_max, s_tmp->pid);
+ ppid_max = MAX(ppid_max, s_tmp->ppid);
+ s_tmp++;
+ }
+
+ pid_digits_max = MAX(pid_digits_max, count_digits(pid_max));
+ ppid_digits_max = MAX(ppid_digits_max, count_digits(ppid_max));
+
/* what info of the processes is shown */
- printf(OPT_BATCH_MODE ? "%.*s" : ESC"[7m" "%.*s" ESC"[m", scr_width,
- " PID PPID USER STAT VSZ %VSZ"
+ snprintf(top_title_str, scr_width + 1,
+ "%*s%s %*s%s %s",
+ pid_digits_max - (int)strlen(PID_STR), " ", PID_STR,
+ ppid_digits_max - (int)strlen(PPID_STR), " ", PPID_STR,
+ "USER STAT VSZ %VSZ"
IF_FEATURE_TOP_SMP_PROCESS(" CPU")
IF_FEATURE_TOP_CPU_USAGE_PERCENTAGE(" %CPU")
" COMMAND");
+ printf(OPT_BATCH_MODE ? "%.*s" : ESC"[7m" "%.*s" ESC"[m", scr_width, top_title_str);
lines_rem--;
#if ENABLE_FEATURE_TOP_DECIMALS
@@ -689,8 +733,6 @@ static NOINLINE void display_process_list(int lines_rem, int scr_width)
lines_rem = ntop - G_scroll_ofs;
s = top + G_scroll_ofs;
while (--lines_rem >= 0) {
- int n;
- char *ppu;
char ppubuf[sizeof(int)*3 * 2 + 12];
char vsz_str_buf[8];
unsigned col;
@@ -702,36 +744,16 @@ static NOINLINE void display_process_list(int lines_rem, int scr_width)
smart_ulltoa5(s->vsz, vsz_str_buf, " mgtpezy");
/* PID PPID USER STAT VSZ %VSZ [%CPU] COMMAND */
- n = sprintf(ppubuf, "%5u %5u %-8.8s", s->pid, s->ppid, get_cached_username(s->uid));
- ppu = ppubuf;
- if (n != 6+6+8) {
- /* Format PID PPID USER part into 6+6+8 chars:
- * shrink PID/PPID if possible, then truncate USER
- */
- char *p, *pp;
- if (*ppu == ' ') {
- do {
- ppu++, n--;
- if (n == 6+6+8)
- goto shortened;
- } while (*ppu == ' ');
- }
- pp = p = skip_non_whitespace(ppu) + 1;
- if (*p == ' ') {
- do
- p++, n--;
- while (n != 6+6+8 && *p == ' ');
- overlapping_strcpy(pp, p); /* shrink PPID */
- }
- ppu[6+6+8] = '\0'; /* truncate USER */
- }
- shortened:
+ snprintf(ppubuf, sizeof(ppubuf), "%*u %*u %-8.8s",
+ pid_digits_max, s->pid, ppid_digits_max, s->ppid,
+ get_cached_username(s->uid));
+
col = snprintf(line_buf, scr_width,
"\n" "%s %s %.5s" FMT
IF_FEATURE_TOP_SMP_PROCESS(" %3d")
IF_FEATURE_TOP_CPU_USAGE_PERCENTAGE(FMT)
" ",
- ppu,
+ ppubuf,
s->state, vsz_str_buf,
SHOW_STAT(pmem)
IF_FEATURE_TOP_SMP_PROCESS(, s->last_seen_on_cpu)
--
2.30.2
_______________________________________________
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
