From owner-svn-src-head@FreeBSD.ORG Wed Aug 21 21:37:58 2013 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTP id EE618F6D; Wed, 21 Aug 2013 21:37:57 +0000 (UTC) (envelope-from jilles@stack.nl) Received: from mx1.stack.nl (relay02.stack.nl [IPv6:2001:610:1108:5010::104]) (using TLSv1 with cipher ADH-CAMELLIA256-SHA (256/256 bits)) (No client certificate requested) by mx1.freebsd.org (Postfix) with ESMTPS id 99D102E4E; Wed, 21 Aug 2013 21:37:57 +0000 (UTC) Received: from snail.stack.nl (snail.stack.nl [IPv6:2001:610:1108:5010::131]) by mx1.stack.nl (Postfix) with ESMTP id 85697358C60; Wed, 21 Aug 2013 23:37:55 +0200 (CEST) Received: by snail.stack.nl (Postfix, from userid 1677) id 6C01028494; Wed, 21 Aug 2013 23:37:55 +0200 (CEST) Date: Wed, 21 Aug 2013 23:37:55 +0200 From: Jilles Tjoelker To: Sergey Kandaurov Subject: Re: svn commit: r254600 - head/lib/libutil Message-ID: <20130821213755.GA8052@stack.nl> References: <201308211646.r7LGk6eV051215@svn.freebsd.org> <5214F72B.7070006@freebsd.org> <20130821190309.GB52908@omg> <20130821202725.GA4991@stack.nl> <20130821212413.GC52908@omg> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20130821212413.GC52908@omg> User-Agent: Mutt/1.5.21 (2010-09-15) Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org, Andrey Chernov X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 21 Aug 2013 21:37:58 -0000 On Thu, Aug 22, 2013 at 01:24:13AM +0400, Sergey Kandaurov wrote: > On Wed, Aug 21, 2013 at 10:27:25PM +0200, Jilles Tjoelker wrote: > > On Wed, Aug 21, 2013 at 11:03:10PM +0400, Sergey Kandaurov wrote: > > > On Wed, Aug 21, 2013 at 09:21:47PM +0400, Andrey Chernov wrote: > > > > On 21.08.2013 20:46, Sergey Kandaurov wrote: > > > > > number = strtoumax(buf, &endptr, 0); > > > > > > > > > > + if (number == UINTMAX_MAX && errno == ERANGE) { > > > > > + return (-1); > > > > > + } > > > > You need to reset errno before strtoumax() call (errno = 0), because any > > > > of previous functions may left it as ERANGE. > > > Thanks for pointing out. > > > Does the patch look good? > > > Index: expand_number.c > > > =================================================================== > > > --- expand_number.c (revision 254600) > > > +++ expand_number.c (working copy) > > > @@ -53,6 +53,8 @@ > > > unsigned shift; > > > char *endptr; > > > > > > + errno = 0; > > > + > > > number = strtoumax(buf, &endptr, 0); > > > > > > if (number == UINTMAX_MAX && errno == ERANGE) { > > This may cause the function to set errno=0 if it is successful, which is > > not allowed for standard library functions from C and POSIX. There may > > be a problem not only if expand_number() is standardized but also if it > > is used in the implementation of a standard library function. The best > > solution is to save and restore errno around this (if [ERANGE] is > > detected, that is a valid errno value to keep). > > In an application it is acceptable to set errno=0 without further ado. > What about this change? > It changes errno only if it was modified from zero in strtoumax(). > Unconditionally restoring errno after strtoumax() unless ERANGE is > probably not good as strtoumax() might set different errno (e.g. EINVAL) > and we might want to keep it as well. Please correct me, if I'm wrong. > Index: lib/libutil/expand_number.c > =================================================================== > --- lib/libutil/expand_number.c (revision 254600) > +++ lib/libutil/expand_number.c (working copy) > @@ -50,15 +50,22 @@ > expand_number(const char *buf, uint64_t *num) > { > uint64_t number; > + int saved_errno; > unsigned shift; > char *endptr; > > + saved_errno = errno; > + errno = 0; > + > number = strtoumax(buf, &endptr, 0); > > if (number == UINTMAX_MAX && errno == ERANGE) { > return (-1); > } > > + if (errno == 0) > + errno = saved_errno; > + > if (endptr == buf) { > /* No valid digits. */ > errno = EINVAL; > This looks good to me. -- Jilles Tjoelker