svn commit: r333945 - head/usr.bin/top

[Bruce Evans]

On Fri, 25 May 2018, Eitan Adler wrote:

> On 20 May 2018 at 23:39, Alexey Dokuchaev <danfe at freebsd.org> wrote:
>> On Mon, May 21, 2018 at 10:32:30AM +1000, Bruce Evans wrote:
>>> ...
>>>>     if (smpmode && namelength > SMPUNAMELEN)
>>>>             namelength = SMPUNAMELEN;
>>>>     else if (namelength > UPUNAMELEN)
>>>
> ...
>
> what about this?

It is now too simple.

> commit 7d041879b4d0ad11818b5f5875b1198a722841d7
> Author: Eitan Adler <lists at eitanadler.com>
> Date:   Sat May 26 04:30:48 2018 +0000
>
>    top(1): allow to configure the max username length
>
>    Some users prefer shorter names, and MAXLOGNAME is not a perfect method
>    of coming up with a default. Use 8, by request, and allow users to
>    configure it.

8 was the (default) minimum name length, but this makes it the (default)
maximum.  This is a surprising change...

> diff --git a/usr.bin/top/machine.c b/usr.bin/top/machine.c
> index 0f31d87..d6a9b41 100644
> --- a/usr.bin/top/machine.c
> +++ b/usr.bin/top/machine.c
> @@ -50,8 +50,9 @@
> #include "layout.h"
>
> #define GETSYSCTL(name, var) getsysctl(name, &(var), sizeof(var))
> -#define SMPUNAMELEN 13
> -#define UPUNAMELEN 15
> +#ifndef MAXTOPNAMELEN
> +#define MAXTOPNAMELEN 8
> +#endif

13 was not too bad as a default maximum.  15 was bogus.  Most users use
SMP so would never see 15 being used.

The extra 2 for UP is to waste space made available by not having a column
for the CPU number.  But width 2 for this field broke a year or 2 ago on
large systems with more than 99 CPUs.  The format string for the CPU number
is still hard-coded to " %2d".

Several fields are formatted correctly with variable width.  The USER field
is one of these.

>
> extern struct timeval timeout;
> static int smpmode;
> @@ -329,11 +330,7 @@ machine_init(struct statics *statics)
>     NULL, 0) == 0 && carc_en == 1)
>  carc_enabled = 1;
>
> - namelength = MAXLOGNAME;
> - if (smpmode && namelength > SMPUNAMELEN)
> - namelength = SMPUNAMELEN;
> - else if (namelength > UPUNAMELEN)
> - namelength = UPUNAMELEN;
> + namelength = MAXTOPNAMELEN;
>
>  kd = kvm_open(NULL, _PATH_DEVNULL, NULL, O_RDONLY, "kvm_open");
>  if (kd == NULL)

I don't like having many run-time configuration options, but one for this
would work well.  Default to either 8 or 13 so as to not surprise too many
users.  But have a runtime option to set change the default.  This is almost
as easy as having a compile-time option (don't have both).  It takes just
1 letter in the getopt string and 3 lines in a case statement for top's
usual sloppy arg parsing (case X; namelength = atoi(optarg); break;).
Documentation takes about the same as for a compile-time option.

The old code would have been efficient enough on most systems it it
had stopped trying to find the longest name after finding one that is
as long as the maximum.  To avoid long scans on large systems with a
silly order of user name (like sorting the shortest ones first), stop
after about 100 names.

I think adjusting the format to match the width of the USER field is
done correctly.  Thus dynamically reducing the width to much less than
8 is already useful for getting more columns in the COMMAND field.
AFAIK there is no way to control the widths of the other fields.
Certainly not the CPU number field.

Bruce