From owner-freebsd-hackers@FreeBSD.ORG Tue Jan 4 12:27:01 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 056E61065670 for ; Tue, 4 Jan 2011 12:27:01 +0000 (UTC) (envelope-from keramida@ceid.upatras.gr) Received: from igloo.linux.gr (igloo.linux.gr [62.1.205.36]) by mx1.freebsd.org (Postfix) with ESMTP id 88AA78FC18 for ; Tue, 4 Jan 2011 12:27:00 +0000 (UTC) X-Spam-Status: No X-Hellug-MailScanner-From: keramida@ceid.upatras.gr X-Hellug-MailScanner-SpamCheck: not spam, SpamAssassin (not cached, score=-2.9, required 5, autolearn=not spam, ALL_TRUSTED -1.00, BAYES_00 -1.90) X-Hellug-MailScanner: Found to be clean X-Hellug-MailScanner-ID: p04CQktb029436 Received: from gkeramidas-glaptop.linux.gr ([74.125.57.36]) (authenticated bits=0) by igloo.linux.gr (8.14.3/8.14.3/Debian-9.4) with ESMTP id p04CQktb029436 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NOT); Tue, 4 Jan 2011 14:26:52 +0200 From: Giorgos Keramidas To: Kostik Belousov References: <6BEFA669-2E1F-44E6-897A-0A51DA939A74@gmail.com> <20110102101845.GC90883@deviant.kiev.zoral.com.ua> <20110104112502.GM3140@deviant.kiev.zoral.com.ua> Date: Tue, 04 Jan 2011 13:26:40 +0100 In-Reply-To: <20110104112502.GM3140@deviant.kiev.zoral.com.ua> (Kostik Belousov's message of "Tue, 4 Jan 2011 13:25:02 +0200") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.0.50 (gnu/linux) MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha1; protocol="application/pgp-signature" 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 12:27:01 -0000 --=-=-= Content-Type: text/plain On Tue, 4 Jan 2011 13:25:02 +0200, Kostik Belousov wrote: >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: >>> 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: >>> >>> /* >>> * Parse an integer from 'string' into 'value'. Return the first >>> * invalid character in 'endp', if endp is non-NULL. The return value >>> * of parseint is 0 on success, -1 for any parse error. >>> */ >>> >>> int >>> parseint(const char *string, const char **endp, int base, int *value) >>> ... > } > > Well, I think it shall be simplified. What about this ? This looks much better than plain strtol() calls. Thanks :) > 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$"); > > #include > #include > -#include > > #include > #include > +#include > #include > #include > #include > #include > > -static void usage(); > +static int parseint(const char *, const char *); > +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 *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 +70,12 @@ main(argc, argv) > > switch (argc) { > case 2: > - proc = abs(atoi(argv[1])); /* Should check if numeric > - * arg! */ > + proc = parseint(argv[1], "pid"); > + 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: > @@ -103,19 +103,17 @@ main(argc, argv) > usage(); > break; > } > - } else { > - rtp.prio = atoi(argv[1]); > - } > + } else > + rtp.prio = parseint(argv[1], "priority"); > } else { > usage(); > break; > } > > if (argv[2][0] == '-') > - proc = -atoi(argv[2]); > - > + proc = parseint(argv[2] + 1, "pid"); > 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 +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 = 0; > + res = strtol(str, &endp, 10); > + if (errno != 0 || endp == str || *endp != '\0') > + err(1, "%s shall be a number", errname); > + if (res >= INT_MAX) > + err(1, "Integer overflow parsing %s", errname); > + return (res); > } > > 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", --=-=-= Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.10 (GNU/Linux) iEYEARECAAYFAk0jEgAACgkQ1g+UGjGGA7aeSACffIzcQAu+rwUHEmSx4t2bMVeY iUcAn3MZ9Dqdb7uuC7Bf7I1luDY7sc7+ =PXOi -----END PGP SIGNATURE----- --=-=-=--