fix large PID overflow their column in top command

[Matthew Chae]

Hi Bernhard,

I'm sending new patch and the result of bloatcheck.
Can you review these and give me your thoughts?
Just let me know if you think that the code size needs to be reduced.

Br-Matthew
________________________________
From: Bernhard Reutner-Fischer <rep.dot.nop at gmail.com>
Sent: Tuesday, February 13, 2024 7:16 PM
To: Matthew Chae <Matthew.Chae at axis.com>
Cc: rep.dot.nop at gmail.com <rep.dot.nop at gmail.com>; David Laight <David.Laight at ACULAB.COM>; 'Denys Vlasenko' <vda.linux at googlemail.com>; busybox at busybox.net <busybox at busybox.net>; Christopher Wong <Christopher.Wong at axis.com>
Subject: Re: fix large PID overflow their column in top command

On Mon, 5 Feb 2024 09:56:20 +0000
Matthew Chae <Matthew.Chae at axis.com> wrote:

> 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?

Unfortunately the patch does not apply to current master.
How much would your patch add to the size? Can you bring it down to a
minimum?
See make baseline; apply the patch;make bloatcheck

thanks

> (0002-Allocate-PID-PPID-space-dynamically-in-top-command.patch)
>
> BR-Matthew Chae
> ________________________________
> From: David Laight <David.Laight at ACULAB.COM>
> Sent: Thursday, November 23, 2023 6:10 PM
> To: 'Denys Vlasenko' <vda.linux at googlemail.com>; Matthew Chae <Matthew.Chae at axis.com>
> Cc: busybox at busybox.net <busybox at busybox.net>; Christopher Wong <Christopher.Wong at 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)

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/busybox/attachments/20240214/228c3cf7/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: bloatcheck_top
Type: application/octet-stream
Size: 1643 bytes
Desc: bloatcheck_top
URL: <http://lists.busybox.net/pipermail/busybox/attachments/20240214/228c3cf7/attachment-0001.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-Allocate-PID-PPID-space-dynamically-in-top-command.patch
Type: text/x-patch
Size: 4493 bytes
Desc: 0003-Allocate-PID-PPID-space-dynamically-in-top-command.patch
URL: <http://lists.busybox.net/pipermail/busybox/attachments/20240214/228c3cf7/attachment-0001.bin>