Re: [PATCH v1 1/2] xstrtol: Correctly handle an invalid base

[Alejandro Colomar]

Hi Paul,

On Fri, Jul 19, 2024 at 02:53:38PM GMT, Alejandro Colomar wrote:
> strtol(3) doesn't set the end pointer if the base is invalid.  This
> allows a caller to differentiate between "invalid base" (what
> strtoi(3bsd) calls EINVAL) and an "no digits seen" (what strtoi(3bsd)
> calls ECANCELED) in systems that report EINVAL on no digits seen (POSIX
> allows this).
> 
>       strtol("foo", &e, 0);
>       strtol("0", &e, -1);
> 
> The former call will set e = nptr.
> The latter will leave e untouched.
> 
> The caller has no other way to portably differentiate the calls.
> 
> The way to differentiate those, thus, is to initialize e = NULL, to
> allow reading it after the call.
> 
> While doing this, change the behavior of this function to only set
> *endptr if strtol(3) has set it, leaving it untouched otherwise.
> 
> Fixes: 034a18049cbc (2014-12-20, "assure: new module")

Self-correction: I had a typo while running git-blame(1), which led me
to not find that the assure() call already existed --as assert()--
before your commit, Paul.

It was actually introduced in 3af47ca3f67 (1994-12-20, ".").

Please amend the Fixes tags to:

        Fixes: 3af47ca3f67 (1994-12-20, ".")
        Fixes: 64ddc975e72c (2024-07-18, "xstrtol: document and stray less from 
strtol")

Sorry for the confusion!

Cheers,
Alex

> Fixes: 64ddc975e72c (2024-07-18, "xstrtol: document and stray less from 
> strtol")
> Cc: Paul Eggert <eggert@cs.ucla.edu>
> Cc: Bruno Haible <bruno@clisp.org>
> Cc: Đoàn Trần Công Danh <congdanhqx@gmail.com>
> Cc: Eli Schwartz <eschwartz93@gmail.com>
> Cc: Sam James <sam@gentoo.org>
> Cc: Serge Hallyn <serge@hallyn.com>
> Cc: Iker Pedrosa <ipedrosa@redhat.com>
> Cc: Michael Vetter <jubalh@iodoru.org>
> Cc: <liba2i@lists.linux.dev>
> Signed-off-by: Alejandro Colomar <alx@kernel.org>
> ---
>  lib/xstrtol.c | 35 +++++++++++++++++++++--------------
>  1 file changed, 21 insertions(+), 14 deletions(-)
> 
> diff --git a/lib/xstrtol.c b/lib/xstrtol.c
> index c3145171f3..592673557f 100644
> --- a/lib/xstrtol.c
> +++ b/lib/xstrtol.c
> @@ -71,9 +71,7 @@ strtol_error
>  __xstrtol (char const *nptr, char **endptr, int base,
>             __strtol_t *val, char const *valid_suffixes)
>  {
> -  char *t_ptr;
> -  char **p = endptr ? endptr : &t_ptr;
> -  *p = (char *) nptr;
> +  char *e = NULL;
>  
>    if (! TYPE_SIGNED (__strtol_t))
>      {
> @@ -82,14 +80,21 @@ __xstrtol (char const *nptr, char **endptr, int base,
>        while (isspace (ch))
>          ch = *++q;
>        if (ch == '-')
> -        return LONGINT_INVALID;
> +        {
> +          if (endptr)
> +            *endptr = (char *) nptr;
> +          return LONGINT_INVALID;
> +        }
>      }
>  
>    errno = 0;
> -  __strtol_t tmp = __strtol (nptr, p, base);
> +  __strtol_t tmp = __strtol (nptr, &e, base);
>    strtol_error err = LONGINT_OK;
>  
> -  if (*p == nptr)
> +  if (endptr && e)
> +    *endptr = e;
> +
> +  if (e == nptr)
>      {
>        /* If there is no number but there is a valid suffix, assume the
>           number is 1.  The string is invalid otherwise.  */
> @@ -113,19 +118,19 @@ __xstrtol (char const *nptr, char **endptr, int base,
>        return err;
>      }
>  
> -  if (**p != '\0')
> +  if (*e != '\0')
>      {
>        int xbase = 1024;
>        int suffixes = 1;
>        strtol_error overflow;
>  
> -      if (!strchr (valid_suffixes, **p))
> +      if (!strchr (valid_suffixes, *e))
>          {
>            *val = tmp;
>            return err | LONGINT_INVALID_SUFFIX_CHAR;
>          }
>  
> -      switch (**p)
> +      switch (*e)
>          {
>          case 'E': case 'G': case 'g': case 'k': case 'K': case 'M': case 'm':
>          case 'P': case 'Q': case 'R': case 'T': case 't': case 'Y': case 'Z':
> @@ -138,10 +143,10 @@ __xstrtol (char const *nptr, char **endptr, int base,
>               power-of-1024.  */
>  
>            if (strchr (valid_suffixes, '0'))
> -            switch (p[0][1])
> +            switch (e[1])
>                {
>                case 'i':
> -                if (p[0][2] == 'B')
> +                if (e[2] == 'B')
>                    suffixes += 2;
>                  break;
>  
> @@ -153,7 +158,7 @@ __xstrtol (char const *nptr, char **endptr, int base,
>                }
>          }
>  
> -      switch (**p)
> +      switch (*e)
>          {
>          case 'b':
>            overflow = bkm_scale (&tmp, 512);
> @@ -224,8 +229,10 @@ __xstrtol (char const *nptr, char **endptr, int base,
>          }
>  
>        err |= overflow;
> -      *p += suffixes;
> -      if (**p)
> +      e += suffixes;
> +      if (endptr)
> +        *endptr = e;
> +      if (*e)
>          err |= LONGINT_INVALID_SUFFIX_CHAR;
>      }
>  
> -- 
> 2.45.2
> 



-- 
<https://www.alejandro-colomar.es/>

signature.asc
Description: PGP signature