Hi Denys,

I'm sending the patch what I've mentioned. Can you take a look at this?
If I don't understand correctly what you said before, let me know.

Have a nice weekend,
Matthew Chae
________________________________
From: busybox <busybox-boun...@busybox.net> on behalf of Matthew Chae 
<matthew.c...@axis.com>
Sent: Wednesday, November 8, 2023 5:33 PM
To: Denys Vlasenko <vda.li...@googlemail.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

Hi Denys,

Ok. Good to hear.
If I understand correctly, will you first review my patch and then give me 
Morris's account to commit the change?
Then, let me send a patch soon.

Br-Matthew
________________________________
From: Denys Vlasenko <vda.li...@googlemail.com>
Sent: Wednesday, November 8, 2023 12:35 PM
To: 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

On Wed, Nov 1, 2023 at 10:18 AM Matthew Chae <matthew.c...@axis.com> wrote:
>
> Hi maintainer,
>
> I'd like to hear your opinion regarding large PID can overflow its column in 
> top command.
>
> The max PID value can have 7 digits as the 4194304.
> Busybox v1.36 released a new patch to handle large PID value in top command.
> However, it still causes misalignment between the title and the number for 7 
> digits or 6 digits PID.
> 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.
> Plus,  the large PID causes that user name can't be displayed up to 8 
> characters and result in the truncation of less than 8 characters.
>
> I can contribute new patch which allocates appropriate space for the number 
> of digits in the PID and PPID to represent the values in each column 
> correctly.
> This can make alignment between the title and the number for 7 digits PID and 
> 5 digits PID and also display user name up to 8 characters.
>
> What do you think? If you agree, I will send a patch.

Absolutely, please send a patch. Would be interesting to see a good solution.
From 42cafd2bbbdc37ef19ddec7f09311f13d83cc957 Mon Sep 17 00:00:00 2001
From: Matthew Chae <matth...@axis.com>
Date: Fri, 10 Nov 2023 15:18:52 +0100
Subject: [PATCH] fix large PID overflow their column 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 appropriate space based on the
max number of PID to accurately represent the values in each column,
also ensuring that user names are displayed without truncation, up to 8
characters.

Signed-off-by: Matthew Chae <matth...@axis.com>
---
 procps/top.c | 61 +++++++++++++++++++++++++---------------------------
 1 file changed, 29 insertions(+), 32 deletions(-)

diff --git a/procps/top.c b/procps/top.c
index 6d25d9633..f158c8ae1 100644
--- a/procps/top.c
+++ b/procps/top.c
@@ -604,6 +604,8 @@ static unsigned long display_header(int scr_width, int *lines_rem_p)
 
 static NOINLINE void display_process_list(int lines_rem, int scr_width)
 {
+#define PID_DIGITS_MAX 7 /* 4194304 */
+
 	enum {
 		BITS_PER_INT = sizeof(int) * 8
 	};
@@ -637,12 +639,30 @@ typedef struct { unsigned quot, rem; } bb_div_t;
 # define FMT "%4u%%"
 #endif
 
+	char pid_buf[PID_DIGITS_MAX + 1];
+	char top_title_str[60];
+	unsigned int pid_digits_num;
+	FILE *fp;
+
+	fp = xfopen_for_read("/proc/sys/kernel/pid_max");
+	if (!fgets(pid_buf, PID_DIGITS_MAX + 1, fp)) {
+		fclose(fp);
+		bb_simple_perror_msg_and_die("can't read standard input");
+	}
+	fclose(fp);
+
+	if (strncmp(pid_buf, "32768", 5) == 0)
+		pid_digits_num = 5;
+	else
+		pid_digits_num = PID_DIGITS_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"
-		IF_FEATURE_TOP_SMP_PROCESS(" CPU")
-		IF_FEATURE_TOP_CPU_USAGE_PERCENTAGE(" %CPU")
-		" COMMAND");
+	sprintf(top_title_str, "%*sPID %*sPPID USER     STAT   VSZ %VSZ",
+		pid_digits_num - 3, " ", pid_digits_num - 4, " ");
+	strcat(top_title_str, 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--;
 
 	/*
@@ -696,8 +716,6 @@ typedef struct { unsigned quot, rem; } bb_div_t;
 		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;
@@ -709,36 +727,15 @@ typedef struct { unsigned quot, rem; } bb_div_t;
 
 		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_num, s->pid, pid_digits_num, 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

Reply via email to