On Tue, 30 Oct 2018 21:09:29 +0100, Klemens Nanni wrote:

> This introduces filteruser() and filterpid(), where the first one can
> now easily be extended to support matching numeric UIDs. The latter one
> is now used by the `highlight` command as well.

I really dislike the side-effect in filteruser(), see below.
Otherwise looks good.

 - todd

> Index: top.c
> ===================================================================
> RCS file: /cvs/src/usr.bin/top/top.c,v
> retrieving revision 1.94
> diff -u -p -r1.94 top.c
> --- top.c     5 Oct 2018 18:56:57 -0000       1.94
> +++ top.c     30 Oct 2018 19:56:07 -0000
> @@ -83,7 +83,7 @@ int no_command = Yes;
>  int old_system = No;
>  int old_threads = No;
>  int show_args = No;
> -pid_t hlpid = -1;
> +pid_t hlpid = (pid_t)-1;
>  int combine_cpus = 0;
>  
>  #if Default_TOPN == Infinity
> @@ -131,6 +131,46 @@ usage(void)
>           __progname);
>  }
>  
> +static int
> +filteruser(char buf[])
> +{
> +     char *bufp = buf;
> +     uid_t *uidp;
> +
> +     if (bufp[0] == '-') {
> +             bufp++[0] = ' ';

Why is this needed, can't you just do "bufp++"?

> +             uidp = &ps.huid;
> +             ps.uid = (pid_t)-1;
> +     } else {
> +             uidp = &ps.uid;
> +             ps.huid = (pid_t)-1;
> +     }
> +
> +     return uid_from_user(bufp, uidp);
> +}
> +
> +static int
> +filterpid(char buf[], int hl)
> +{
> +     const char *errstr;
> +     int pid;
> +
> +     pid = strtonum(buf, 0, INT_MAX, &errstr);
> +     if (errstr != NULL || !find_pid(pid))
> +             return -1;
> +
> +     if (hl == Yes)
> +             hlpid = (pid_t)pid;
> +     else {
> +             if (ps.system == No)
> +                     old_system = No;
> +             ps.pid = (pid_t)pid;
> +             ps.system = Yes;
> +     }
> +
> +     return 0;
> +}
> +
>  static void
>  parseargs(int ac, char **av)
>  {

Reply via email to