Date: Tue, 4 Jan 2011 13:25:02 +0200 From: Kostik Belousov <kostikbel@gmail.com> To: Giorgos Keramidas <keramida@freebsd.org> Cc: Eitan Adler <lists@eitanadler.com>, hackers@freebsd.org Subject: Re: [patch] have rtprio check that arguments are numeric; change atoi to strtol Message-ID: <20110104112502.GM3140@deviant.kiev.zoral.com.ua> In-Reply-To: <xeia7heluf2q.fsf@kobe.laptop> 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> <xeiad3oduf9l.fsf@kobe.laptop> <xeia7heluf2q.fsf@kobe.laptop>
next in thread | previous in thread | raw e-mail | index | archive | help
--OGW1Z2JKiS9bXo17
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
On Tue, Jan 04, 2011 at 11:40:45AM +0100, Giorgos Keramidas wrote:
> On Tue, 04 Jan 2011 11:36:38 +0100, Giorgos Keramidas <keramida@freebsd.o=
rg> wrote:
> > On Sun, 2 Jan 2011 18:46:47 -0500, Eitan Adler <lists@eitanadler.com> w=
rote:
> >> 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 fir=
st
> > * invalid character in 'endp', if endp is non-NULL. The retur=
n 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 =3D 0;
> > x =3D strtol(string, endp, base);
> > if (errno !=3D 0 || endp =3D=3D str || (endp !=3D NULL =
&&
> > endp !=3D str && *endp !=3D '\0' && (isdigit(*endp)=
=3D=3D 0 ||
> > isspace(*endp) !=3D 0)))
> > return -1;
> > if (x >=3D INT_MAX) {
> > errno =3D ERANGE;
> > return -1;
> > }
> > return (int)x;
> > }
>=20
> instead of `return (int)x' the last bits should be slightly different,
> of course:
>=20
> if (value !=3D NULL)
> *value =3D (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$");
=20
#include <sys/param.h>
#include <sys/rtprio.h>
-#include <sys/errno.h>
=20
#include <ctype.h>
#include <err.h>
+#include <errno.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
=20
-static void usage();
+static int parseint(const char *, const char *);
+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 *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 +70,12 @@ main(argc, argv)
=20
switch (argc) {
case 2:
- proc =3D abs(atoi(argv[1])); /* Should check if numeric
- * arg! */
+ proc =3D parseint(argv[1], "pid");
+ 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:
@@ -103,19 +103,17 @@ main(argc, argv)
usage();
break;
}
- } else {
- rtp.prio =3D atoi(argv[1]);
- }
+ } else
+ rtp.prio =3D parseint(argv[1], "priority");
} else {
usage();
break;
}
=20
if (argv[2][0] =3D=3D '-')
- proc =3D -atoi(argv[2]);
-
+ proc =3D parseint(argv[2] + 1, "pid");
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 +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 =3D 0;
+ res =3D strtol(str, &endp, 10);
+ if (errno !=3D 0 || endp =3D=3D str || *endp !=3D '\0')
+ err(1, "%s shall be a number", errname);
+ if (res >=3D INT_MAX)
+ err(1, "Integer overflow parsing %s", errname);
+ return (res);
}
=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",
--OGW1Z2JKiS9bXo17
Content-Type: application/pgp-signature
Content-Disposition: inline
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (FreeBSD)
iEYEARECAAYFAk0jA40ACgkQC3+MBN1Mb4jWbACg8/dLTPu1y3BtiADPqDyzcxsE
5Y4AnAyT3okHzF2IF1yEkJ8pf8Xr9oDe
=MFsG
-----END PGP SIGNATURE-----
--OGW1Z2JKiS9bXo17--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20110104112502.GM3140>