svn commit: r254600 - head/lib/libutil

[Sergey Kandaurov]

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;

-------------- next part --------------
A non-text attachment was scrubbed...
Name: expand_number.2.diff
Type: text/x-diff
Size: 607 bytes
Desc: not available
URL: <http://lists.freebsd.org/pipermail/svn-src-head/attachments/20130822/ac6f157c/attachment.diff>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.freebsd.org/pipermail/svn-src-head/attachments/20130822/ac6f157c/attachment.sig>