On Tue, 4 Jan 2011 13:25:02 +0200, Kostik Belousov <[email protected]> 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 <[email protected]> >>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 <sys/param.h>
> #include <sys/rtprio.h>
> -#include <sys/errno.h>
>
> #include <ctype.h>
> #include <err.h>
> +#include <errno.h>
> #include <stdio.h>
> #include <stdlib.h>
> #include <string.h>
> #include <unistd.h>
>
> -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",
pgp01wK7MOYI9.pgp
Description: PGP signature
