On Sun, Jan 02, 2011 at 06:46:47PM -0500, Eitan Adler wrote:
> What about this patch? I incorporated your feedback so I am not going
> to reply inline.
>
> > The syntax of the prio commands is weird, there is an obvious corner
> > (or wrongly handled) case, where the command name starts with a digit.
> > Command name starting with dash is even harder.
> >
>
> I agree - and I wouldn't mind seeing the syntax changed (along with
> the licensed changed to a 2 clause BSD license) - but I suspect the
> benefits conferred by those two things would not be enough to overcome
> the reluctance to change a very old command (since 1994). If I'm wrong
> I'll gladly write a "cleanroom" version with sane syntax.
>
> Index: rtprio.c
> ===================================================================
> --- rtprio.c (revision 216679)
> +++ rtprio.c (working copy)
> @@ -41,6 +41,7 @@
>
> #include <ctype.h>
> #include <err.h>
> +#include <libgen.h>
> #include <stdio.h>
> #include <stdlib.h>
> #include <string.h>
> @@ -54,14 +55,12 @@
> char **argv;
> {
> char *p;
> - int proc = 0;
> + pid_t proc;
> struct rtprio rtp;
> + char *invalidchar;
>
> - /* find basename */
> - if ((p = rindex(argv[0], '/')) == NULL)
> - p = argv[0];
> - else
> - ++p;
> + proc = 0;
> + p = basename(argv[0]);
>
> if (!strcmp(p, "rtprio"))
> rtp.type = RTP_PRIO_REALTIME;
> @@ -70,8 +69,10 @@
>
> switch (argc) {
> case 2:
> - proc = abs(atoi(argv[1])); /* Should check if numeric
> - * arg! */
> + proc = (int)strtol(argv[1], &invalidchar, 10);
> + if (*invalidchar != '\0')
> + errx(1,"Process should be a pid");
> + proc = abs(proc);
> /* FALLTHROUGH */
> case 1:
> if (rtprio(RTP_LOOKUP, proc, &rtp) != 0)
> @@ -104,16 +105,20 @@
> break;
> }
> } else {
> - rtp.prio = atoi(argv[1]);
> + rtp.prio = (int)strtol(argv[1], &invalidchar,
> 10);
> + if (*invalidchar != '\0')
> + errx(1,"Priority should be a number",
> invalidchar);
This did not compiled for me.
> }
> } else {
> usage();
> break;
> }
>
> - if (argv[2][0] == '-')
> - proc = -atoi(argv[2]);
> -
> + if (argv[2][0] == '-') {
> + proc = (int)strtol(argv[2]+1, &invalidchar, 10);
> + if (*invalidchar != '\0')
> + errx(1,"Process should be a pid");
> + }
> if (rtprio(RTP_SET, proc, &rtp) != 0)
> err(1, "%s", argv[0]);
>
>
Unless loud objections are heard, I indend to commit the following patch,
based on your submission.
diff --git a/usr.sbin/rtprio/rtprio.c b/usr.sbin/rtprio/rtprio.c
index 245f714..bdd61ea 100644
--- a/usr.sbin/rtprio/rtprio.c
+++ b/usr.sbin/rtprio/rtprio.c
@@ -37,7 +37,6 @@ __FBSDID("$FreeBSD$");
#include <sys/param.h>
#include <sys/rtprio.h>
-#include <sys/errno.h>
#include <ctype.h>
#include <err.h>
@@ -46,22 +45,21 @@ __FBSDID("$FreeBSD$");
#include <string.h>
#include <unistd.h>
-static void usage();
+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 *invalidchar, *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 +68,14 @@ main(argc, argv)
switch (argc) {
case 2:
- proc = abs(atoi(argv[1])); /* Should check if numeric
- * arg! */
+ proc = strtol(argv[1], &invalidchar, 10);
+ if (*invalidchar != '\0' || invalidchar == argv[1])
+ errx(1, "Pid should be a number");
+ 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:
@@ -104,18 +104,23 @@ main(argc, argv)
break;
}
} else {
- rtp.prio = atoi(argv[1]);
+ rtp.prio = strtol(argv[1], &invalidchar, 10);
+ if (*invalidchar != '\0' ||
+ invalidchar == argv[1])
+ errx(1, "Priority should be a number");
}
} else {
usage();
break;
}
- if (argv[2][0] == '-')
- proc = -atoi(argv[2]);
-
+ if (argv[2][0] == '-') {
+ proc = strtol(argv[2] + 1, &invalidchar, 10);
+ if (*invalidchar != '\0' || invalidchar == argv[2] + 1)
+ errx(1, "Pid should be a number");
+ }
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 +128,13 @@ main(argc, argv)
}
exit(0);
}
- exit (1);
+ exit(1);
}
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",
pgp2RR1ZSETol.pgp
Description: PGP signature
