Re: [PATCH 1/5] xen/lib: Move simple_strtoul from common/vsprintf.c to lib

[Jan Beulich <jbeulich@xxxxxxxx>]

On 28.07.2023 23:35, Shawn Anastasio wrote:
> Move the simple_strtoul routine which is used throughout the codebase
> from vsprintf.c to its own file in xen/lib.
> 
> This allows libfdt to be built on ppc64 even though xen/common doesn't
> build yet.
> 
> Signed-off-by: Shawn Anastasio <sanastasio@xxxxxxxxxxxxxxxxxxxxx>
> ---
>  xen/common/vsprintf.c    | 37 -------------------------------------
>  xen/lib/Makefile         |  1 +
>  xen/lib/simple_strtoul.c | 40 ++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 41 insertions(+), 37 deletions(-)
>  create mode 100644 xen/lib/simple_strtoul.c

What about its siblings? It'll be irritating to find one here and the
other there.

Also please no underscores in (new) filenames unless there's a reason
for this. In the case here, though, I question the need for "simple"
in the file name in the first place.

> --- /dev/null
> +++ b/xen/lib/simple_strtoul.c
> @@ -0,0 +1,40 @@
> +/*
> + *  Copyright (C) 1991, 1992  Linus Torvalds
> + */
> +
> +#include <xen/ctype.h>
> +
> +/**
> + * simple_strtoul - convert a string to an unsigned long
> + * @cp: The start of the string
> + * @endp: A pointer to the end of the parsed string will be placed here
> + * @base: The number base to use
> + */
> +unsigned long simple_strtoul(
> +    const char *cp, const char **endp, unsigned int base)
> +{
> +    unsigned long result = 0,value;
> +
> +    if (!base) {
> +        base = 10;
> +        if (*cp == '0') {
> +            base = 8;
> +            cp++;
> +            if ((toupper(*cp) == 'X') && isxdigit(cp[1])) {
> +                cp++;
> +                base = 16;
> +            }
> +        }
> +    } else if (base == 16) {
> +        if (cp[0] == '0' && toupper(cp[1]) == 'X')
> +            cp += 2;
> +    }
> +    while (isxdigit(*cp) &&
> +           (value = isdigit(*cp) ? *cp-'0' : toupper(*cp)-'A'+10) < base) {
> +        result = result*base + value;
> +        cp++;
> +    }
> +    if (endp)
> +        *endp = cp;
> +    return result;
> +}

While moving, I think it would be nice if this stopped using neither
Xen nor Linux style. I'm not going to insist, but doing such style
adjustments right here would be quite nice.

Jan