Hi Paul,
> > + strtol_error s_err = __xstrtol (input, &endp, -1, &val, "k");
> > + ASSERT (s_err == LONGINT_INVALID);
> > + ASSERT (endp == NULL);
> > + ASSERT (val == -17);
>
> This isn't a valid test case. If the base is -1, the underlying strtol
> can support any base notation it likes. It might, for example, support
> base 64 and the "k" would be valid input.
The purpose of this unit test is to formalize and check expectations.
Just like we have
ASSERT (errno == EINVAL || errno == EBUSY || errno == EISDIR
|| errno == ENOTEMPTY || errno == EEXIST
|| errno == ENOENT /* WSL */);
in some other tests.
If a platform comes along and implements a behaviour like you describe,
this unit test will alert us of it, and we can ponder what to do.
So, the existence of such a unit test is a feature.
> If the documentation doesn't make it sufficiently clear that the
> behavior is not fully specified with an invalid base, we can do that.
The unit test and the documentation complement each other:
- The unit test provides precision details that the documentation is
not clear about.
- The unit test is also the basis for writing documentation (as we've
seen in the 'xstrtod' case yesterday).
More fundamentally, though, I have a hard time understanding the point
and purpose of what you are saying:
- In <https://lists.gnu.org/archive/html/bug-gnulib/2024-07/msg00171.html>
you removed the validation of 'base', saying:
"strtol can support base 1 as an extension, xstrtol now does
whatever the system does".
- In <https://lists.gnu.org/archive/html/bug-gnulib/2024-07/msg00238.html>
you agreed to leaving it unspecified whether xstrtol sets *endptr
in some case.
If strtol has a corner with unspecified behaviour, I don't think we should
replicate this corner with unspecified behaviour in Gnulib APIs, because that
does not serve our users:
- It leaves programmer pitfalls open, instead of preventing them.
- It leaves portability problems open, instead of fixing them.
We are not building Rust bindings to libc functions. We are building Gnulib
APIs with the goal to help developers. For this purpose, I don't feel that
even documentation like "If you pass a base other than 0 or 2..36, you
can't expect a particular parsing, nor a particular error code, nor a
particular value for *endptr." is good. It is better to prevent such
unspecified behaviour, for the reasons listed above.
Bruno
