On Tue, Jan 04, 2011 at 11:40:45AM +0100, Giorgos Keramidas wrote: > On Tue, 04 Jan 2011 11:36:38 +0100, Giorgos Keramidas <[email protected]> > wrote: > > On Sun, 2 Jan 2011 18:46:47 -0500, Eitan Adler <[email protected]> wrote: > >> What about this patch? I incorporated your feedback so I am not going > >> to reply inline. > > > > Since the pattern of converting strings to int-derivative values appears > > multiple times, I'd probably prefer something like a new function that > > does the parsing *and* error-checking, to avoid duplication of errno > > checks, invalidchar checks, and so on, e.g. something like this near the > > top of rtprio.c: > > > > #include <errno.h> > > #include <limits.h> > > #include <string.h> > > > > /* > > * Parse an integer from 'string' into 'value'. Return the first > > * invalid character in 'endp', if endp is non-NULL. The return > > value > > * of parseint is 0 on success, -1 for any parse error. > > */ > > > > int > > parseint(const char *string, const char **endp, int base, int > > *value) > > { > > long x; > > > > errno = 0; > > x = strtol(string, endp, base); > > if (errno != 0 || endp == str || (endp != NULL && > > endp != str && *endp != '\0' && (isdigit(*endp) == 0 || > > isspace(*endp) != 0))) > > return -1; > > if (x >= INT_MAX) { > > errno = ERANGE; > > return -1; > > } > > return (int)x; > > } > > instead of `return (int)x' the last bits should be slightly different, > of course: > > if (value != NULL) > *value = (int)x; > return 0; > } Well, I think it shall be simplified. What about this ?
diff --git a/usr.sbin/rtprio/rtprio.c b/usr.sbin/rtprio/rtprio.c
index 245f714..fc212b5 100644
--- a/usr.sbin/rtprio/rtprio.c
+++ b/usr.sbin/rtprio/rtprio.c
@@ -37,31 +37,31 @@ __FBSDID("$FreeBSD$");
#include <sys/param.h>
#include <sys/rtprio.h>
-#include <sys/errno.h>
#include <ctype.h>
#include <err.h>
+#include <errno.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
-static void usage();
+static int parseint(const char *, const char *);
+static void usage(void);
int
-main(argc, argv)
- int argc;
- char **argv;
+main(int argc, char *argv[])
{
- char *p;
- int proc = 0;
struct rtprio rtp;
+ char *p;
+ pid_t proc;
/* find basename */
if ((p = rindex(argv[0], '/')) == NULL)
p = argv[0];
else
++p;
+ proc = 0;
if (!strcmp(p, "rtprio"))
rtp.type = RTP_PRIO_REALTIME;
@@ -70,12 +70,12 @@ main(argc, argv)
switch (argc) {
case 2:
- proc = abs(atoi(argv[1])); /* Should check if numeric
- * arg! */
+ proc = parseint(argv[1], "pid");
+ proc = abs(proc);
/* FALLTHROUGH */
case 1:
if (rtprio(RTP_LOOKUP, proc, &rtp) != 0)
- err(1, "%s", argv[0]);
+ err(1, "RTP_LOOKUP");
printf("%s: ", p);
switch (rtp.type) {
case RTP_PRIO_REALTIME:
@@ -103,19 +103,17 @@ main(argc, argv)
usage();
break;
}
- } else {
- rtp.prio = atoi(argv[1]);
- }
+ } else
+ rtp.prio = parseint(argv[1], "priority");
} else {
usage();
break;
}
if (argv[2][0] == '-')
- proc = -atoi(argv[2]);
-
+ proc = parseint(argv[2] + 1, "pid");
if (rtprio(RTP_SET, proc, &rtp) != 0)
- err(1, "%s", argv[0]);
+ err(1, "RTP_SET");
if (proc == 0) {
execvp(argv[2], &argv[2]);
@@ -123,12 +121,28 @@ main(argc, argv)
}
exit(0);
}
- exit (1);
+ exit(1);
+}
+
+static int
+parseint(const char *str, const char *errname)
+{
+ char *endp;
+ long res;
+
+ errno = 0;
+ res = strtol(str, &endp, 10);
+ if (errno != 0 || endp == str || *endp != '\0')
+ err(1, "%s shall be a number", errname);
+ if (res >= INT_MAX)
+ err(1, "Integer overflow parsing %s", errname);
+ return (res);
}
static void
-usage()
+usage(void)
{
+
(void) fprintf(stderr, "%s\n%s\n%s\n%s\n%s\n%s\n",
"usage: [id|rt]prio",
" [id|rt]prio [-]pid",
pgpRlkNXRGHNm.pgp
Description: PGP signature
