Re: [patch] ps: atol(3) -> strtonum(3)

2016-03-08 Thread fritjof
On Tue, Mar 08, 2016 at 01:31:47PM -0700, Todd C. Miller wrote:
> On Tue, 08 Mar 2016 09:32:25 -0700, Theo de Raadt wrote:
> 
> > atol maps to strtol.  Which can accept hex or octal.
> 
> atol() maps to strtol(str, (char **)NULL, 10) so this is a good
> place to use strtonum().  The max value should probably be PID_MAX
> rather than INT_MAX but we only expose that #ifdef _KERNEL.
>

This is what I figured out as well.

>  - todd
> 
X

--F.



Re: [patch] ps: atol(3) -> strtonum(3)

2016-03-08 Thread Todd C. Miller
On Tue, 08 Mar 2016 09:32:25 -0700, Theo de Raadt wrote:

> atol maps to strtol.  Which can accept hex or octal.

atol() maps to strtol(str, (char **)NULL, 10) so this is a good
place to use strtonum().  The max value should probably be PID_MAX
rather than INT_MAX but we only expose that #ifdef _KERNEL.

 - todd



Re: [patch] ps: atol(3) -> strtonum(3)

2016-03-08 Thread Theo de Raadt
atol maps to strtol.  Which can accept hex or octal.

strtonum cannot do that.

So you have missed a step here -- a justification based upon
verification that loss of support for octal/hex is a good step
forward.  Basically, some archeology.  You need to go pull on a string
for a while, then report back on positive finds, conversations about
it, historical use of octal/hex, etc.

> Index: ps.c
> ===
> RCS file: /cvs/src/bin/ps/ps.c,v
> retrieving revision 1.63
> diff -u -r1.63 ps.c
> --- ps.c  16 Jan 2015 06:39:32 -  1.63
> +++ ps.c  8 Mar 2016 14:15:34 -
> @@ -99,6 +99,7 @@
>   int all, ch, flag, i, fmt, lineno, nentries;
>   int prtheader, showthreads, wflag, kflag, what, Uflag, xflg;
>   char *nlistf, *memf, *swapf, *cols, errbuf[_POSIX2_LINE_MAX];
> + const char *errstr;
>  
>   if ((cols = getenv("COLUMNS")) != NULL && *cols != '\0') {
>   const char *errstr;
> @@ -189,7 +190,9 @@
>   fmt = 1;
>   break;
>   case 'p':
> - pid = atol(optarg);
> + pid = strtonum(optarg, 1, INT_MAX, );
> + if (errstr != NULL)
> + errx(1, "%s is an invalid pid", optarg);
>   xflg = 1;
>   break;
>   case 'r':
>