[patch] have rtprio check that arguments are numeric; change atoi to strtol

[Giorgos Keramidas]

On Tue, 4 Jan 2011 13:25:02 +0200, Kostik Belousov <kostikbel at gmail.com> 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 <keramida at freebsd.org> 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",
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
Url : http://lists.freebsd.org/pipermail/freebsd-hackers/attachments/20110104/e707543b/attachment.pgp