From: Alexander Bluhm <bluhm@openbsd.org>
Subject: Re: usr.bin/patch: use strtonum instead of atoi()
To: Omar Polo <op@omarpolo.com>
Cc: tech@openbsd.org
Date: Thu, 29 Aug 2024 10:42:49 +0200

On Thu, Aug 29, 2024 at 12:42:18AM +0200, Omar Polo wrote:
> atoi() is only used to parse numbers from arguments, so there's no
> trailing garbage.  As a bonus, we reject negative values which makes no
> sense.  INT_MAX is just an upper limit.  way too big.
> 
> oks?

s/fuzz factor/maximum fuzz/
s/debug value/debug number/

This is closer to the wording in the man page.

OK bluhm@

> diff 5a1af3cffe24ad9d9a983a89b5a57b51a5cc18b3 b30480ea2d6abd9e55cb8ea416d02bb6060dd225
> commit - 5a1af3cffe24ad9d9a983a89b5a57b51a5cc18b3
> commit + b30480ea2d6abd9e55cb8ea416d02bb6060dd225
> blob - 96da0572381e10ed35a0196c6425fc0a5ec4f0c6
> blob + 30f164cf922efc833a4fa48cdf1f847f977744ff
> --- usr.bin/patch/patch.c
> +++ usr.bin/patch/patch.c
> @@ -542,6 +542,7 @@ get_some_switches(void)
>  		{NULL,			0,			0,	0}
>  	};
>  	int ch;
> +	const char *errstr;
>  
>  	rejname[0] = '\0';
>  	Argc_last = Argc;
> @@ -598,7 +599,10 @@ get_some_switches(void)
>  			force = true;
>  			break;
>  		case 'F':
> -			maxfuzz = atoi(optarg);
> +			maxfuzz = strtonum(optarg, 0, INT_MAX, &errstr);
> +			if (errstr != NULL)
> +				fatal("fuzz factor is %s: %s\n",
> +				    errstr, optarg);
>  			break;
>  		case 'i':
>  			if (++filec == MAXFILEC)
> @@ -618,7 +622,10 @@ get_some_switches(void)
>  			outname = xstrdup(optarg);
>  			break;
>  		case 'p':
> -			strippath = atoi(optarg);
> +			strippath = strtonum(optarg, 0, INT_MAX, &errstr);
> +			if (errstr != NULL)
> +				fatal("strip count is %s: %s\n",
> +				    errstr, optarg);
>  			break;
>  		case 'r':
>  			if (strlcpy(rejname, optarg,
> @@ -647,7 +654,10 @@ get_some_switches(void)
>  			break;
>  #ifdef DEBUGGING
>  		case 'x':
> -			debug = atoi(optarg);
> +			debug = strtonum(optarg, 0, INT_MAX, &errstr);
> +			if (errstr != NULL)
> +				fatal("debug value is %s: %s\n",
> +				    errstr, optarg);
>  			break;
>  #endif
>  		default: