Date: Mon, 3 Jan 2011 14:48:05 +0200 From: Kostik Belousov <kostikbel@gmail.com> To: Eitan Adler <lists@eitanadler.com> Cc: hackers@freebsd.org Subject: Re: [patch] have rtprio check that arguments are numeric; change atoi to strtol Message-ID: <20110103124805.GB3140@deviant.kiev.zoral.com.ua> In-Reply-To: <AANLkTinfKYYy0s3_LLvgRZtx=hGjbZP4%2Bqtnk31oi9GP@mail.gmail.com> References: <AANLkTimiJPiHBSw5i5TVJYfh9uGOyrNJx%2BoUPeB%2Bt%2BY_@mail.gmail.com> <6BEFA669-2E1F-44E6-897A-0A51DA939A74@gmail.com> <AANLkTimmQHRoQOZBgfheds=_sv5SMzD9DrPfYk9em7j8@mail.gmail.com> <AANLkTimdEpZ1DBoKU9g6w-j=F9KS0ONvo9racgOesdPf@mail.gmail.com> <20110102101845.GC90883@deviant.kiev.zoral.com.ua> <AANLkTinfKYYy0s3_LLvgRZtx=hGjbZP4%2Bqtnk31oi9GP@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
--yEPQxsgoJgBvi8ip
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
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.
>=20
> > 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.
> >
>=20
> 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.
>=20
> Index: rtprio.c
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> --- rtprio.c (revision 216679)
> +++ rtprio.c (working copy)
> @@ -41,6 +41,7 @@
>=20
> #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 =3D 0;
> + pid_t proc;
> struct rtprio rtp;
> + char *invalidchar;
>=20
> - /* find basename */
> - if ((p =3D rindex(argv[0], '/')) =3D=3D NULL)
> - p =3D argv[0];
> - else
> - ++p;
> + proc =3D 0;
> + p =3D basename(argv[0]);
>=20
> if (!strcmp(p, "rtprio"))
> rtp.type =3D RTP_PRIO_REALTIME;
> @@ -70,8 +69,10 @@
>=20
> switch (argc) {
> case 2:
> - proc =3D abs(atoi(argv[1])); /* Should check if numeric
> - * arg! */
> + proc =3D (int)strtol(argv[1], &invalidchar, 10);
> + if (*invalidchar !=3D '\0')
> + errx(1,"Process should be a pid");
> + proc =3D abs(proc);
> /* FALLTHROUGH */
> case 1:
> if (rtprio(RTP_LOOKUP, proc, &rtp) !=3D 0)
> @@ -104,16 +105,20 @@
> break;
> }
> } else {
> - rtp.prio =3D atoi(argv[1]);
> + rtp.prio =3D (int)strtol(argv[1], &invalidchar, 10);
> + if (*invalidchar !=3D '\0')
> + errx(1,"Priority should be a number", invalidchar);
This did not compiled for me.
> }
> } else {
> usage();
> break;
> }
>=20
> - if (argv[2][0] =3D=3D '-')
> - proc =3D -atoi(argv[2]);
> -
> + if (argv[2][0] =3D=3D '-') {
> + proc =3D (int)strtol(argv[2]+1, &invalidchar, 10);
> + if (*invalidchar !=3D '\0')
> + errx(1,"Process should be a pid");
> + }
> if (rtprio(RTP_SET, proc, &rtp) !=3D 0)
> err(1, "%s", argv[0]);
>=20
>=20
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$");
=20
#include <sys/param.h>
#include <sys/rtprio.h>
-#include <sys/errno.h>
=20
#include <ctype.h>
#include <err.h>
@@ -46,22 +45,21 @@ __FBSDID("$FreeBSD$");
#include <string.h>
#include <unistd.h>
=20
-static void usage();
+static void usage(void);
=20
int
-main(argc, argv)
- int argc;
- char **argv;
+main(int argc, char *argv[])
{
- char *p;
- int proc =3D 0;
struct rtprio rtp;
+ char *invalidchar, *p;
+ pid_t proc;
=20
/* find basename */
if ((p =3D rindex(argv[0], '/')) =3D=3D NULL)
p =3D argv[0];
else
++p;
+ proc =3D 0;
=20
if (!strcmp(p, "rtprio"))
rtp.type =3D RTP_PRIO_REALTIME;
@@ -70,12 +68,14 @@ main(argc, argv)
=20
switch (argc) {
case 2:
- proc =3D abs(atoi(argv[1])); /* Should check if numeric
- * arg! */
+ proc =3D strtol(argv[1], &invalidchar, 10);
+ if (*invalidchar !=3D '\0' || invalidchar =3D=3D argv[1])
+ errx(1, "Pid should be a number");
+ proc =3D abs(proc);
/* FALLTHROUGH */
case 1:
if (rtprio(RTP_LOOKUP, proc, &rtp) !=3D 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 =3D atoi(argv[1]);
+ rtp.prio =3D strtol(argv[1], &invalidchar, 10);
+ if (*invalidchar !=3D '\0' ||
+ invalidchar =3D=3D argv[1])
+ errx(1, "Priority should be a number");
}
} else {
usage();
break;
}
=20
- if (argv[2][0] =3D=3D '-')
- proc =3D -atoi(argv[2]);
-
+ if (argv[2][0] =3D=3D '-') {
+ proc =3D strtol(argv[2] + 1, &invalidchar, 10);
+ if (*invalidchar !=3D '\0' || invalidchar =3D=3D argv[2] + 1)
+ errx(1, "Pid should be a number");
+ }
if (rtprio(RTP_SET, proc, &rtp) !=3D 0)
- err(1, "%s", argv[0]);
+ err(1, "RTP_SET");
=20
if (proc =3D=3D 0) {
execvp(argv[2], &argv[2]);
@@ -123,12 +128,13 @@ main(argc, argv)
}
exit(0);
}
- exit (1);
+ exit(1);
}
=20
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",
--yEPQxsgoJgBvi8ip
Content-Type: application/pgp-signature
Content-Disposition: inline
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (FreeBSD)
iEYEARECAAYFAk0hxYUACgkQC3+MBN1Mb4h6cACg7A4GWm+t1TMTbztERjYCGxmz
C+oAnjfjNTCOIWmasShQ3+d0tUfwbn62
=ZNty
-----END PGP SIGNATURE-----
--yEPQxsgoJgBvi8ip--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20110103124805.GB3140>