On Fri, Jul 19, 2024 at 12:14:19AM GMT, Alejandro Colomar wrote:
> [CC -= Andrew, per explicit request]
>
> On Thu, Jul 18, 2024 at 11:25:11PM GMT, Alejandro Colomar wrote:
> > On Thu, Jul 18, 2024 at 11:09:40PM GMT, Bruno Haible wrote:
> > > Hi Alejandro,
>
> Hi Bruno,
>
> > > > strtol(3) has a limited set of possible states:
> > > > ...
> > > > The condition '*endp != s && errno != 0 && errno != ERANGE' is
> > > > unreachable. The only errno possible if '*endp != s' is ERANGE.
> > >
> > > Such a statement can be true if you look at the standards (ISO C, POSIX).
> > >
> > > However, there's a difference between what the standards say and what the
> > > systems actually do. The Gnulib documentation contains thousands of
> > > examples
> > > of such differences.
> > >
> > > Gnulib therefore (almost) never assumes that there are no possible errno
> > > values besides the ones listed in the standards.
> > > - Some systems return "wrong" errno values. Example: [1]
> > > - Some systems fail with ENOMEM when memory is tight. Who says that
> > > an implementation of strtol() cannot use malloc() ? Some
> > > implementations
> > > of strtod() do use malloc().
> > >
> > > So, what you call "dead code", I call "defensive programming". I would not
> > > like to apply this patch.
>
> On the other hand, I'm not sure that defensive programming is valid in
> this case. I already discussed this topic with Serge (but we didn't
> have in mind any specific implementation) some time ago, and,
>
> We'd need to know the precise specification of that system that can set
> errno = ENOMEM.
>
> Is *endp guaranteed to be set? Or may it be unset (as happens with
> EINVAL)?
>
> If it is allowed to keep *endp unset, then the first `if (*p == s)`
> would already be UB. And for truly being defensive, we'd need to assume
> that it may leave *endp unset. Thus we cannot read that until we know
> that no errno has been set. But then comes the problem that some
> systems set EINVAL on what strtoi(3) calls ECANCELED. To work with all
> of those systems, we'd probably need to pass a dummy NULL to make sure
> we can inspect *endp regardless of strtol(3) having set it or not:
>
> intmax_t
> strtoi(char *s, char **restrict endp, int base,
> intmax_t min, intmax_t max, int *restrict status)
> {
> int errno_saved, st;
> char *e;
> char **ep;
> intmax_t n;
>
> e = NULL;
> ep = &e;
>
> if (status == NULL)
> status = &st;
>
> if (base != 0 && (base < 2 || base > 36)) {
> *status = EINVAL;
> return MAX(min, MIN(max, 0));
> }
>
> errno_saved = errno;
> errno = 0;
>
> n = strtoimax(s, ep, base);
>
> if (errno == ERANGE || n < min || n > max)
> *status = ERANGE;
> else if (e == s && (errno == 0 || errno == EINVAL))
> *status = ECANCELED;
> else if (errno != 0)
> *status = errno;
> else if (*e != '\0')
> *status = ENOTSUP;
> else
> *status = 0;
Hmm, bug there. I should have written:
if (errno == ERANGE)
*status = ERANGE;
else if (e == s && (errno == 0 || errno == EINVAL))
*status = ECANCELED;
else if (errno != 0)
*status = errno;
else if (n < min || n > max)
*status = ERANGE;
else if (*e != '\0')
*status = ENOTSUP;
else
*status = 0;
>
> errno = errno_saved;
>
> if (endp != NULL)
> *endp = e;
>
> return MAX(min, MIN(max, n));
> }
>
> Does this make sense?
>
> Have a lovely night!
> Alex
>
> >
> > Makes sense. I think we should document that possibility in the manual
> > page. Maybe say that other errno values are possible in some systems?
> > Otherwise, it's already a hell of a function to take care of, and most
> > uses don't handle that possibility at the moment. (Yet more reasons to
> > use a wrapper that returns -1 & sets errno on error, as the rest of
> > libc.)
> >
> > Would you send a patch? (I'd write it myself, but you probably can
> > provide more info in the commit message.)
> >
> > Have a lovely night!
> > Alex
> >
> > >
> > > Bruno
> > >
> > > [1]
> > > https://www.gnu.org/software/gnulib/manual/html_node/getlogin_005fr.html
> > >
> > >
> > >
> >
> > --
> > <https://www.alejandro-colomar.es/>
>
>
>
> --
> <https://www.alejandro-colomar.es/>
--
<https://www.alejandro-colomar.es/>
signature.asc
Description: PGP signature
