Re: Reuse strtonum(3) and reallocarray(3) from OpenBSD

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

Christos Zoulas wrote:
> With the helpful feedback from rmind, joerg, etc. I've written a function
> which I call strtoi() (and strtou()) which tries to fix the API issues with
> strtonum() and provides strtonum() as a wrapper:
> 
> /* 
>  * in inttypes.h:
>  */
> intmax_t strtoi(const char * __restrict, char ** __restrict, int,
>     intmax_t, intmax_t, int *);
> uintmax_t strtou(const char * __restrict, char ** __restrict, int,
>     uintmax_t, uintmax_t, int *);
> 

Hello

Once again, many thanks Christos, Jorge and Mindaugas!

I was poked for discussing design on netbsd-bugs@ (hello David!).. so to resume the
discussion I ask questions regarding API design.

I will play the Devil's advocate (hopefully constructive).

1)
Is errno usage allowed only for libc functions? If not then why not
drop the last parameter and replace it with errno?

It's a responsibility of a developer to save or handle errno.

2)
The proposed strtoi(3) returns information of partial conversion
twice: in endptr and in rerror (kind of imitation of errno on demand).

It's duplication of information.

3)
Partial conversion returns error (copied from errno(2)):

     86 ENOTSUP Not supported. An attempt was made to set or change a
             parameter to an unsupported value.

Does it match well? Maybe 22 EINVAL describes the things better:

     22 EINVAL Invalid argument. Some invalid argument was supplied.  (For
             example, specifying an undefined signal to a signal(3) or kill(2)
             function).

... however partial conversion isn't "Not supported" neither "Invalid argument".

To ilustrate it, there's an example of strtod(3):
/* strtod example */
#include <stdio.h>      /* printf, NULL */
#include <stdlib.h>     /* strtod */

int main ()
{
  char szOrbits[] = "365.24 29.53";
  char* pEnd;
  double d1, d2;
  d1 = strtod (szOrbits, &pEnd);
  d2 = strtod (pEnd, NULL);
  printf ("The moon completes %.2f orbits per Earth year.\n", d1/d2);
  return 0;
}

http://www.cplusplus.com/reference/cstdlib/strtod/
If we replace strtod with strtoi (and adjust the numbers) strtoi(3) will emit rerror/errno for the first translation ENOTSUP (yes, we can ignore it... but it's kind of flaw to me).

4)
The original intention of strtonum(3) and probably majority cases when it will be used is when
a developer wants to just translate only one number from a string to a single integer. In case
of partial conversion handle the overall operation as failed.

5) In this API there are many input parameters... 4 of strtonum(3) vs 6 of strtoi(3).

To summarize I don't think that people wanting to just have the job done will like strtoi(3) better than strto(3)...

My counter proposition is to use errno "as God intended" and split strtoi(3) to two functions (kind of low and high):
- strtoi(3) with partial conversion handling
- fullstrtoi(3) without partial conversion handling, just try to translate the whole string and shut up

This split makes the code more clean, see how these functions are now specialized at doing one thing well.
Also fullstrtoi(3), being the direct equivalent to strtonum(3), takes the same number of parameters (4 vs 4).
Our fullstrtoi will be better than strtonum(3) having dynamic base support, and so it will be a replacement of strtonum() from ZFS (it's changing hex string to integers).

Here is the proposed API with an embedded example (compile and test as home-work).

#include <stdio.h>
#include <errno.h>
#include <inttypes.h>
#include <string.h>

/*
 * If no error occurs, errno is left unchanged. This behavior
 * (which is unlike most library functions) is guaranteed by pertinent
 * standards of strtol(3)-like functions.
 */

intmax_t
strtoi(const char * __restrict ptr, char ** __restrict eptr, int base,
    intmax_t lo, intmax_t hi)
{
	int saved_errno;
	intmax_t im;
	char *ep;

	if (eptr == NULL)
		eptr = &ep;

	saved_errno = errno;
	errno = 0;
	im = strtoimax(ptr, eptr, base);

	if (im < lo) {
		errno = ERANGE;
		return lo;
	}
	if (im > hi) {
		errno = ERANGE;
		return hi;
	}

	errno = saved_errno;
	return im;
}

intmax_t
fullstrtoi(const char * ptr, int base, intmax_t lo, intmax_t hi)
{
	int saved_errno;
	intmax_t im;
	char *ep;

	saved_errno = errno;
	errno = 0;
	im = strtoi(ptr, &ep, base, lo, hi);

	if (ptr == '\0' || *ep != '\0')
		errno = EINVAL;

	if (errno == 0)
		errno = saved_errno;
	return im;
}

int main ()
{
  char szOrbits[] = "365 29";
  char* pEnd;
  int d1, d2, d3;

  d1 = strtoi (szOrbits, &pEnd, 0, 0, 1000);
  printf("errno: %s\n", strerror(errno));
  d2 = strtoi (pEnd, NULL, 0, 0, 1000);
  printf("errno: %s\n", strerror(errno));
  printf ("The moon completes %d orbits per Earth year.\n", d1/d2);

  d3 = fullstrtoi(szOrbits, 0, 0, 1000);
  printf("errno: %s\n", strerror(errno));

  return 0;
}