[Toybox] [patch] add renice

[Felix Janda]

Some comments below.

Strake wrote:
> From feeb53bf11e949e48f80752a642fe551c284d7a7 Mon Sep 17 00:00:00 2001
> From: Strake <strake888 at gmail.com>
> Date: Sun, 28 Jul 2013 09:30:35 -0500
> Subject: [PATCH] add renice
> 
> ---
>  toys/pending/renice.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 48 insertions(+)
>  create mode 100644 toys/pending/renice.c
> 
> diff --git a/toys/pending/renice.c b/toys/pending/renice.c
> new file mode 100644
> index 0000000..44219c2
> --- /dev/null
> +++ b/toys/pending/renice.c
> @@ -0,0 +1,48 @@
> +/* renice.c - renice process
> + *
> + * Copyright 2013 CE Strake <strake888 at gmail.com>
> + *
> + * See http://pubs.opengroup.org/onlinepubs/9699919799/utilities/
> + * See http://refspecs.linuxfoundation.org/LSB_4.1.0/LSB-Core-generic/LSB-Core-generic/cmdbehav.html
> +
> +USE_RENICE(NEWTOY(renice, "gpun#", TOYFLAG_BIN))
> +
> +config RENICE
> +  bool "renice"
> +  default n
> +  help
> +    usage: renice [-gpu] -n increment ID ...
> +*/
> +
> +#define FOR_renice
> +#include "toys.h"
> +
> +GLOBALS(
> +  long nArgu;
> +)
> +
> +void renice_main (void) {
> +  int ii;
> +  int which = toys.optflags & FLAG_g ? PRIO_PGRP :
> +              toys.optflags & FLAG_u ? PRIO_USER :
> +              PRIO_PROCESS;
> +
> +  if (!(toys.optflags & FLAG_n)) error_exit ("no increment given");

Rob has documented the suffix "|" for the option parsing string to denote
an option which is mandatory. However it is not implemented yet. IMO it
would be useful to comment that if the "|" suffix is implemented at some
point this toy could make use of it.

> +
> +  for (ii = 0; ii < toys.optc; ii++) {

You don't really need the index. Just loop over toys.optargs, which are
conveniently null terminated.

> +    id_t id;
> +
> +    if (isdigit (toys.optargs[ii][0])) id = strtoul (toys.optargs[ii], 0, 10);
> +    else if (toys.optflags & FLAG_u) id = getpwnam (toys.optargs[ii])
> -> pw_uid;

Mail mungling FTW.

This logic looks broken if a username should start with a digit. POSIX says
that if -u is specified, first each argument should be interpreted as a user
name and only then as a decimal ID.

> +    else {
> +      error_msg ("not a number: %s", toys.optargs[ii]);
> +      toys.exitval = 1;
> +      continue;
> +    }
> +
> +    if (setpriority (which, id, getpriority (which, id) + TT.nArgu) < 0) {
> +      error_msg ("failed to setpriority of %d", id);

How about using perror_msg here? errno might contain useful information.

> +      toys.exitval = 1;

Unnecessary here and above. error_msg() calls verror_msg() which sets
toys.exitval to 1 if it was zero (default value) before.

> +    }
> +  }
> +}

 1375046390.0