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

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

Christos Zoulas wrote:
> | 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.
> 
> Then you have strtol() with min/max and read the man page about the
> messiness of handling errno.

Yes, this was my intention to change strtoi(3) to strtol(3) with range checks,
with the feature for manual partial conversion handling.
And add one function with checks for full strting conversion (with leading white spaces).

In general strtol(3) is heavy to use properly, so every step of making it easier to use I consider as benefit.

I took my comment to example from the man page.

> 
> | 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.
> 
> Not necessarily, since *eptr can be NULL or *rerror can be NULL.
> 

We can ignore return values, but the duplication still occurs as there is possibility to check for an event in two distinct ways. For me API should be designed without such possibilities.

> | 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).
> 
> Then you have 3 reasons for returning EINVAL
> 	1 wrong base
> 	2 wrong range
> 	3 partial conversion
> 

In my proposed code there was a bug, I skipped checking for wrong base. Thank you.

1 - wrong base go for EINVAL as default for strtol(3)-like funs
2 - wrong range there is ERANGE always (you was right about it before)
3 - partial conversion hmmm I consider having it for fullstrtoi(3) as valid, but not for the original strtoi(3)

> | 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
> 
> God did not intend to use errno to detect between INTMAX_MAX as overflow and
> INTMAX_MAX as a valid return. This is what makes error handling in strtol()
> complicated, and this is why you have to set errno to 0 before calling
> the function which is highly unusual. You don't set errno to 0 before
> calling each function do you?

I consider losing or floating errno as software bug. Actually I had experience in a larger project that people lost errno and it's floating across the codebase and I had to set it explicitly to 0. And it was used i.a. before strtol(3) call :-(

So to me it's not that unusual.

Please see comment below.

> 
> | 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).
> | 
> 
> Suffers from the original strtol() problem, if lo == INTMAX_MIN or
> hi == INTMAX_MAX, so we've come full cycle.
> christos
> 

No it doesn't suffer. When abstracting information we always lost information.
For those who want to see difference between overflow (ERANGE) and out of range (ERANGE)
there is strtol(3) intended.

My intention is not to make a orchestra function (that even by design must duplicate information),
but to step by step make strtol(3) as simple as atoi(3).
With this direction we lost information with every step, for those who need the details,
there are strtoi(3) and strtol(3).

No full cycle, I see how we make progress! And I started to disliking strtonum(3) seeing what we have now ;-)

Kind regards and the current state of strtoi(3) and fullstrtoi(3) is as follows:

#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;
		im = lo;
	}
	if (im > hi) {
		errno = ERANGE;
		im = hi;
	}

	if (errno == 0)
		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 = ENOTSUP;

	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;
}