From owner-freebsd-hackers@FreeBSD.ORG Tue Jan 4 11:25:08 2011 Return-Path: Delivered-To: hackers@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id ACD48106564A; Tue, 4 Jan 2011 11:25:08 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from mail.zoral.com.ua (mx0.zoral.com.ua [91.193.166.200]) by mx1.freebsd.org (Postfix) with ESMTP id 34A088FC13; Tue, 4 Jan 2011 11:25:07 +0000 (UTC) Received: from deviant.kiev.zoral.com.ua (root@deviant.kiev.zoral.com.ua [10.1.1.148]) by mail.zoral.com.ua (8.14.2/8.14.2) with ESMTP id p04BP2tg020146 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Tue, 4 Jan 2011 13:25:02 +0200 (EET) (envelope-from kostikbel@gmail.com) Received: from deviant.kiev.zoral.com.ua (kostik@localhost [127.0.0.1]) by deviant.kiev.zoral.com.ua (8.14.4/8.14.4) with ESMTP id p04BP2Gw010392; Tue, 4 Jan 2011 13:25:02 +0200 (EET) (envelope-from kostikbel@gmail.com) Received: (from kostik@localhost) by deviant.kiev.zoral.com.ua (8.14.4/8.14.4/Submit) id p04BP2KJ010391; Tue, 4 Jan 2011 13:25:02 +0200 (EET) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: deviant.kiev.zoral.com.ua: kostik set sender to kostikbel@gmail.com using -f Date: Tue, 4 Jan 2011 13:25:02 +0200 From: Kostik Belousov To: Giorgos Keramidas Message-ID: <20110104112502.GM3140@deviant.kiev.zoral.com.ua> References: <6BEFA669-2E1F-44E6-897A-0A51DA939A74@gmail.com> <20110102101845.GC90883@deviant.kiev.zoral.com.ua> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="OGW1Z2JKiS9bXo17" Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.4.2.3i X-Virus-Scanned: clamav-milter 0.95.2 at skuns.kiev.zoral.com.ua X-Virus-Status: Clean X-Spam-Status: No, score=-3.4 required=5.0 tests=ALL_TRUSTED,AWL,BAYES_00, DNS_FROM_OPENWHOIS autolearn=no version=3.2.5 X-Spam-Checker-Version: SpamAssassin 3.2.5 (2008-06-10) on skuns.kiev.zoral.com.ua Cc: Eitan Adler , hackers@freebsd.org Subject: Re: [patch] have rtprio check that arguments are numeric; change atoi to strtol X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 04 Jan 2011 11:25:08 -0000 --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 wrote: > > On Sun, 2 Jan 2011 18:46:47 -0500, Eitan Adler 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 > > #include > > #include > > > > /* > > * 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 #include -#include =20 #include #include +#include #include #include #include #include =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--