From owner-svn-src-all@FreeBSD.ORG Fri Nov 16 11:38:36 2012 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 75F6F44A; Fri, 16 Nov 2012 11:38:36 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail10.syd.optusnet.com.au (mail10.syd.optusnet.com.au [211.29.132.191]) by mx1.freebsd.org (Postfix) with ESMTP id 03BA28FC14; Fri, 16 Nov 2012 11:38:34 +0000 (UTC) Received: from c122-106-175-26.carlnfd1.nsw.optusnet.com.au (c122-106-175-26.carlnfd1.nsw.optusnet.com.au [122.106.175.26]) by mail10.syd.optusnet.com.au (8.13.1/8.13.1) with ESMTP id qAGBcUnI023673 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Fri, 16 Nov 2012 22:38:32 +1100 Date: Fri, 16 Nov 2012 22:38:30 +1100 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Konstantin Belousov Subject: Re: svn commit: r243076 - head/usr.sbin/chkgrp In-Reply-To: <20121115185958.GG73505@kib.kiev.ua> Message-ID: <20121116220347.S1856@besplex.bde.org> References: <201211151506.qAFF63v0003848@svn.freebsd.org> <20121115153030.GD73505@kib.kiev.ua> <20121116032851.I44199@besplex.bde.org> <20121115185958.GG73505@kib.kiev.ua> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed X-Optus-Cloudmark-Score: 0 X-Optus-Cloudmark-Analysis: v=2.0 cv=JdTkNj2V c=1 sm=1 a=IpyuoPnKT5oA:10 a=kj9zAlcOel0A:10 a=PO7r1zJSAAAA:8 a=JzwRw_2MAAAA:8 a=hf-KJMUURdMA:10 a=S9hVmbsOi-6G55ARAw8A:9 a=CjuIK1q_8ugA:10 a=NWVoK91CQyQA:10 a=bxQHXO5Py4tHmhUgaywp5w==:117 Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org, Eitan Adler , Bruce Evans X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 16 Nov 2012 11:38:36 -0000 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 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