Bug 36950
| Summary: | Add -n to renice(8) | ||||||
|---|---|---|---|---|---|---|---|
| Product: | Base System | Reporter: | Peter Avalos <peter> | ||||
| Component: | standards | Assignee: | Tim Robbins <tjr> | ||||
| Status: | Closed FIXED | ||||||
| Severity: | Affects Only Me | ||||||
| Priority: | Normal | ||||||
| Version: | 4.5-STABLE | ||||||
| Hardware: | Any | ||||||
| OS: | Any | ||||||
| Attachments: |
|
||||||
|
Description
Peter Avalos
2002-04-10 05:20:02 UTC
On Wed, Apr 10, 2002 at 04:16:02AM -0000, Peter Avalos wrote: > >Number: 36950 > >Category: standards > >Synopsis: Add -n to renice(8) > Index: renice.8 > =================================================================== > +.Sh STANDARDS > +The > +.Nm > +utility conforms to > +.St -p1003.1-2001 > +Note: Specifying > +.Ar nice_value > +is deprecated, but it is accepted due to its historical significance. The comment about historical behaviour probably belongs in a COMPATIBILITY section. > Index: renice.c > =================================================================== > -static const char copyright[] = > +static char copyright[] = I believe this should stay const. You could consider using the __COPYRIGHT macro, but \n's in its argument don't work properly. > + prio = strtol(*argv, &endptr, 10); > + if (*endptr || Should check whether endptr == *argv. renice '' should not be allowed. > + ((prio == LONG_MAX || prio == LONG_MIN) && errno == ERANGE)) > + errx(EX_DATAERR, "Invalid input: %s", *argv); prio is an int so might not ever be able to take value LONG_{MIN,MAX}. -n should be accepted with its argument joined to it, for example -n4 as well as -n 4. Solaris accepts this and the standard specifies it. NetBSD has got it wrong. > + /* > + * Accept -g, -p, -u, or a number. If it's a number, default > + * to -p (PRIO_PROCESS). > + */ > + if (strcmp(*argv, "-g") == 0) { > + which = PRIO_PGRP; > + argc--, argv++; > + } else if (strcmp(*argv, "-u") == 0) { > + which = PRIO_USER; > + argc--, argv++; > + } else if (strcmp(*argv, "-p") == 0) > + argc--, argv++; > + Handling these in the loop is the traditional BSD, and SUSv2 behaviour. Worth noting in the COMPATIBILITY section of the manual page that this syntax isn't allowed anymore if you choose to remove it. > + /* If argv is not a number, then we should be PRIO_USER. */ No, non-numeric arguments are invalid unless preceeded by -u. > - exit(errs != 0); > + exit(errs); Existing code was better; knowing how many requests failed is not useful, and it could wrap back around to 0 (success) or other strange things. > - printf("%d: old priority %d, new priority %d\n", who, oldprio, prio); > + printf("%d: old nice_value %d, new nice_value %d\n", who, oldprio, prio); This should just be removed alltogether - SUSv3 specifies that standard output is not used at all, despite this being traditional behaviour. Tim Tim J. Robbins <tim@robbins.dropbear.id.au> writes: > On Wed, Apr 10, 2002 at 04:16:02AM -0000, Peter Avalos wrote: > > > Index: renice.c > > =================================================================== > > > -static const char copyright[] = > > +static char copyright[] = > > I believe this should stay const. You could consider using the __COPYRIGHT > macro, but \n's in its argument don't work properly. Vendor copyrights should be left in their original form. Adding `const' here is a necessary evil in order prevent diagnostics on modern compilers. > > - printf("%d: old priority %d, new priority %d\n", who, oldprio, prio); > > + printf("%d: old nice_value %d, new nice_value %d\n", who, oldprio, +prio); > > This should just be removed alltogether - SUSv3 specifies that standard > output is not used at all, despite this being traditional behaviour. I'm not sure of the impact to the existing userbase, but I agree that this diagnostic is very unbecoming (particularly since it's outputted to stdout instead of stderr). Best regards, Mike Barcroft I think things can be done better than what was changed in the Apr. 10
commit, so I re-did the patch against the newest version.
Summary of changes:
- Use FBSDID.
- Use sysexits.
- Change incr to nflag since I think this is better style (originally
mike's idea).
- Change the usage closer to style(9). Along with this, use nice value
since it's less confusing than priority. If I want to raise the
priority, I need to lower the nice value.
- Move more towards POSIX by only accepting 1 option, and then using
that option to determine how the ID is interpreted.
- Allow the user to specify usernames and userids.
- Remove the prio bounds check. setpriority() does this for us.
- Remove the printf that tells the user the old priority and new
priority. POSIX says stdout is not used, and the value of this message
is questionable.
- All numbers should be decimal integers.
Index: renice.c
===================================================================
RCS file: /cvsroot/fbsd/src/usr.bin/renice/renice.c,v
retrieving revision 1.11
diff -u -r1.11 renice.c
--- renice.c 10 Apr 2002 13:38:09 -0000 1.11
+++ renice.c 20 Apr 2002 19:23:42 -0000
@@ -35,16 +35,15 @@
static const char copyright[] =
"@(#) Copyright (c) 1983, 1989, 1993\n\
The Regents of the University of California. All rights reserved.\n";
-#endif /* not lint */
-#ifndef lint
#if 0
static char sccsid[] = "@(#)renice.c 8.1 (Berkeley) 6/9/93";
#endif
-static const char rcsid[] =
- "$FreeBSD: src/usr.bin/renice/renice.c,v 1.11 2002/04/10 13:38:09 maxim Exp $";
#endif /* not lint */
+#include <sys/cdefs.h>
+__FBSDID("$FreeBSD$");
+
#include <sys/types.h>
#include <sys/time.h>
#include <sys/resource.h>
@@ -56,6 +55,7 @@
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
+#include <sysexits.h>
static int donice(int, int, int, int);
static int getnum(const char *, const char *, int *);
@@ -70,38 +70,48 @@
main(int argc, char *argv[])
{
struct passwd *pwd;
- int errs, incr, prio, which, who;
+ char *ep;
+ int errs, nflag, prio, which, who;
errs = 0;
- incr = 0;
+ nflag = 0;
which = PRIO_PROCESS;
who = 0;
argc--, argv++;
if (argc < 2)
usage();
if (strcmp(*argv, "-n") == 0) {
- incr = 1;
+ nflag = 1;
argc--, argv++;
if (argc < 2)
usage();
}
- if (getnum("priority", *argv, &prio))
- return (1);
+ if (getnum("nice_value", *argv, &prio))
+ exit(EX_DATAERR);
argc--, argv++;
+
+ /*
+ * The next argument shall be -g, -p, -u, or an unsigned decimal
+ * number. If it is a number, default to -p (PRIO_PROCESS).
+ */
+ if (strcmp(*argv, "-g") == 0) {
+ which = PRIO_PGRP;
+ argc--, argv++;
+ } else if (strcmp(*argv, "-u") == 0) {
+ which = PRIO_USER;
+ argc--, argv++;
+ } else if (strcmp(*argv, "-p") == 0)
+ argc--, argv++;
+ if (argc < 1)
+ usage();
+
+ /*
+ * From this point on, all non-numeric arguments are invalid unless -u
+ * (PRIO_USER). Numbers shall be unsigned integers.
+ */
for (; argc > 0; argc--, argv++) {
- if (strcmp(*argv, "-g") == 0) {
- which = PRIO_PGRP;
- continue;
- }
- if (strcmp(*argv, "-u") == 0) {
- which = PRIO_USER;
- continue;
- }
- if (strcmp(*argv, "-p") == 0) {
- which = PRIO_PROCESS;
- continue;
- }
- if (which == PRIO_USER) {
+ (void)strtol(*argv, &ep, 10);
+ if (*ep && which == PRIO_USER) {
pwd = getpwnam(*argv);
if (pwd == NULL) {
warnx("%s: unknown user", *argv);
@@ -109,20 +119,20 @@
}
who = pwd->pw_uid;
} else {
- if (getnum("pid", *argv, &who))
+ if (getnum("ID", *argv, &who))
continue;
if (who < 0) {
warnx("%s: bad value", *argv);
continue;
}
}
- errs += donice(which, who, prio, incr);
+ errs += donice(which, who, prio, nflag);
}
exit(errs != 0);
}
static int
-donice(int which, int who, int prio, int incr)
+donice(int which, int who, int prio, int nflag)
{
int oldprio;
@@ -132,17 +142,12 @@
warn("%d: getpriority", who);
return (1);
}
- if (incr)
- prio = oldprio + prio;
- if (prio > PRIO_MAX)
- prio = PRIO_MAX;
- if (prio < PRIO_MIN)
- prio = PRIO_MIN;
+ if (nflag)
+ prio = oldprio + prio; /* Possible over/underflow here. */
if (setpriority(which, who, prio) < 0) {
warn("%d: setpriority", who);
return (1);
}
- printf("%d: old priority %d, new priority %d\n", who, oldprio, prio);
return (0);
}
@@ -153,7 +158,7 @@
char *ep;
errno = 0;
- v = strtol(str, &ep, NULL);
+ v = strtol(str, &ep, 10);
if (v < INT_MIN || v > INT_MAX || errno == ERANGE) {
warnx("%s argument %s is out of range.", com, str);
return (1);
@@ -170,8 +175,8 @@
static void
usage()
{
- fprintf(stderr, "%s\n%s\n",
-"usage: renice [priority | [-n incr]] [ [ -p ] pids ] [ [ -g ] pgrps ]",
-" [ [ -u ] users ]");
- exit(1);
+
+ fprintf(stderr, "usage: renice nice_value [-g | -p | -u] ID ...\n");
+ fprintf(stderr, " renice -n increment [-g | -p | -u] ID ...\n");
+ exit(EX_USAGE);
}
State Changed From-To: open->patched Most of the changes you suggest have now been committed to HEAD in one form or another. Thanks. Responsible Changed From-To: freebsd-standards->tjr I will MFC this when the RELENG_4 code slush ends. State Changed From-To: patched->closed Changes have been MFC'd. |
