Re: lib/49429: Import strtonum(3)

["Kamil Rytarowski" <n54%gmx.com@localhost>]

The following reply was made to PR lib/49429; it has been noted by GNATS.

From: "Kamil Rytarowski" <n54%gmx.com@localhost>
To: gnats-bugs%NetBSD.org@localhost
Cc: christos%netbsd.org@localhost
Subject: Re: lib/49429: Import strtonum(3)
Date: Sat, 27 Dec 2014 03:56:51 +0100

 Christos Zoulas wrote:
 >  | Your solution looks reasonable, but I have few notes:
 >  | 1) the function for (lo > hi) behaves in unpredicted way, I
 >  | suggest set errno immediatetely to EINVAL and return 0 (special
 >  | case)
 >  
 >  I can see putting a _DIAGASSERT() for it, or returning EINVAL.
 >  Still it is not unpredictable; it will always return the same result
 >  for the same value according to the prescribed logic.
 >  
 
 I mean if we call strtoi("123", NULL, 0, 10, 5, NULL) then we get ERANGE and 5 that is not in the valid range.
 
 If someone wants something over 10 and under 5 then there is no valid return value, and I was thinking of fallback to 0.
 
  *	- ERANGE  -> the value returned from the conversion was out of range
  *		     and was corrected to be in range.
 
 The range was invalid.
 
 >  
 >  | 3) the example strtoi("foo", NULL, 0, 10, 20, NULL); makes NULL
 >  | pointer dereference
 >  
 >  Where?
 
 	if (*eptr == NULL)
 
 >  
 >  | 4) do we really want ENOSUP this way?
 >  
 >  We'd like to differentiate between a result failed by strtoimax()
 >  and one that we failed because of our additional checks.
 >  
 
 Sorry, I was wrong here and in the next mail.
 
 >  | 5) if base is not supported then we can unexpectedly abuse errno from EINVAL
 >  | and change return value from 0 to lo/hi
 >  
 >  We wanted base to be supported. Why castrate strtoimax() needlessly?
 >  Anyway even strtonum() does not differentiate; it is all "invalid"
 >  for it.
 >  
 
 Mea culpa, I was blind here and I oversaw `if (*rerror == 0)'. Everything is right!
 
 >  | I'm upset for __OPENBSD_SOURCE, please don't do it!
 >  
 >  Well, you are going to make a compile-time error a link-time error
 >  if you introduce -lobcompat. Anyway we would like to discourage
 >  using functions that we don't like their API so... If you are
 >  compiling our own code, you are not supposed to use strtonum. If
 >  you want 3rd party code to use our own strtonum() you need to state
 >  so.
 >  
 >  | What do you think of libobcompat (OpenBSD compat layer) in
 >  | src/external (and in some appropriate place in pkgsrc), on top of
 >  | libnbcompat, with inline OpenBSD-specific functions. These functions
 >  | will be promised that they are NOT supported by us and they forbidden
 >  | in our own code. The compat layer will be used only to ease having
 >  | external code in the base (OpenSSH, tmux...).
 >  
 >  Functions we like we should add to the base ABI. The ones we do not
 >  like, we should decide what to do rather than cargo-cult them over to
 >  a library just for the sake of compatibility. We definitely do not want
 >  to encourage people adding API's we do not like in the base source.
 >  
 >  | One of the functions, next to strtonum(3) will be explicit_bzero(3).
 >  
 >  Case in point. We already have explicit_memset(3) which is superior to
 >  explicit_bzero(3) because:
 >  
 >  	- it does not build on obsolete APIs (b{copy,zero,cmp} are
 >  	  not obsolete -- politely marked LEGACY in ToG)
 >  	- it has more functionality, allowing to set a value instead
 >  	  of just zero.
 >  
 >  | In all other places, where the code is not external we will switch to
 >  | strtoi(3)/strtou(3).
 >  
 >  Right, and for external code, we can decide; if autoconf is supported
 >  and a replacement is provided we choose. If not, we define
 >  _OPENBSD_SOURCE.
 >  
 >  This is not about NIH; we'll gladly import API's that we consider
 >  robust, well designed, and orthogonal with existing ones.
 >  
 >  christos
 >  
 > 
 
 Well this is applicative, with additional checks for #if HAVE_STRTONUM (and entry in configure.ac).
 
 Thank you!