Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 16 Nov 2012 22:38:30 +1100 (EST)
From:      Bruce Evans <brde@optusnet.com.au>
To:        Konstantin Belousov <kostikbel@gmail.com>
Cc:        svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org, Eitan Adler <eadler@freebsd.org>, Bruce Evans <brde@optusnet.com.au>
Subject:   Re: svn commit: r243076 - head/usr.sbin/chkgrp
Message-ID:  <20121116220347.S1856@besplex.bde.org>
In-Reply-To: <20121115185958.GG73505@kib.kiev.ua>
References:  <201211151506.qAFF63v0003848@svn.freebsd.org> <20121115153030.GD73505@kib.kiev.ua> <CAF6rxgk9w0_Qwo=92g-OSe5imvupG8qg7DzpCS9UVxzwMjn20g@mail.gmail.com> <20121116032851.I44199@besplex.bde.org> <CAF6rxgkvh_QEw5o3-8rznkEQMvQHY=ngYVM-7HQ11jHwX2vZ=w@mail.gmail.com> <20121115185958.GG73505@kib.kiev.ua>

next in thread | previous in thread | raw e-mail | index | archive | help
On Thu, 15 Nov 2012, Konstantin Belousov wrote:

> On Thu, Nov 15, 2012 at 01:52:46PM -0500, Eitan Adler wrote:
>> On 15 November 2012 11:52, Bruce Evans <brde@optusnet.com.au> wrote:
>>> strtoul("1garbage", NULL, 10) succeeds and returns value 1, but the input
>>> is garbage.
>>
>> This case is covered earlier
>> 160         /* check that the GID is numeric */
>> 161         if (strspn(f[2], "0123456789") != strlen(f[2])) {
>> 162             warnx("%s: line %d: GID is not numeric", gfn, n);
> So this code shall be removed, if you are introducing strtoul() to check
> for errors at all.

It is indeed strange to mix methods like this, and the check is more
strict that usual since it doesn't allow leading spaces.  I think leading
spaces are normally allowed for numeric args.  This normally happens
automatically, via bad programs using atoi() and better programs using
strto*().  Extra work like the above has to be done to reject leading
spaces.

>>> As the man page says, the EINVAL feature is unportable.  It is almost
>>> useless, since to detect garbage after the number you have to pass an
>>> endptr to strtoul(), and then the check for no conversion (that is,
>>> for garbage at the beginning) is just as easy as the check for garbage
>>> at the end.
>>
>> This patch doesn't care about EINVAL or ERANGE. It just cares strtoul
>> returned an error.

The only possible error for strtol() on a string of digits is ERANGE.

>> I even considered just ignoring the error case because the data is
>> mostly sanity checked prior.

The prior checking doesn't detect range errors.

The later checking detects range errors on 64-bit systems but not on 32-bit
ones, since strtoul() returns ULONG_MAX for range errors and that is a
valid gid (GID_MAX) on 32-bit systems.

Bruce



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20121116220347.S1856>