From owner-freebsd-hackers@FreeBSD.ORG Mon Jan 3 12:48:20 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 EEC4A10656A4 for ; Mon, 3 Jan 2011 12:48:20 +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 3A64D8FC15 for ; Mon, 3 Jan 2011 12:48:19 +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 p03Cm6YD021987 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Mon, 3 Jan 2011 14:48:06 +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 p03Cm6kD003401; Mon, 3 Jan 2011 14:48:06 +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 p03Cm54b003400; Mon, 3 Jan 2011 14:48:05 +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: Mon, 3 Jan 2011 14:48:05 +0200 From: Kostik Belousov To: Eitan Adler Message-ID: <20110103124805.GB3140@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="yEPQxsgoJgBvi8ip" 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: 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: Mon, 03 Jan 2011 12:48:21 -0000 --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 > #include > +#include > #include > #include > #include > @@ -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 #include -#include =20 #include #include @@ -46,22 +45,21 @@ __FBSDID("$FreeBSD$"); #include #include =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--