Re: [Qemu-devel] [PATCH v4 2/9] cutils: add qemu_strtoi & qemu_strtoui parsers for int/unsigned int types

[Eric Blake <eblake@redhat.com>]

On 02/05/2018 09:24 AM, Daniel P. Berrangé wrote:
> From: "Daniel P. Berrange" <berrange@redhat.com>
> 
> There are qemu_strtoNN functions for various sized integers. This adds two
> more for plain int & unsigned int types, with suitable range checking.
> 
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---

> +++ b/util/cutils.c
> @@ -297,6 +297,110 @@ static int check_strtox_error(const char *nptr, char *ep,
>       return -libc_errno;
>   }
>   
> +/**
> + * Convert string @nptr to a long integer, and store it in @result.

s/a long/an/

> + */
> +int qemu_strtoi(const char *nptr, const char **endptr, int base,
> +                int *result)
> +{
> +    char *ep;
> +    long lresult;
> +
> +    if (!nptr) {
> +        if (endptr) {
> +            *endptr = nptr;
> +        }
> +        return -EINVAL;
> +    }
> +
> +    errno = 0;
> +    lresult = strtol(nptr, &ep, base);
> +    if (lresult < INT_MIN) {
> +        *result = INT_MIN;
> +    } else if (lresult > INT_MAX) {
> +        *result = INT_MAX;

On 64-bit platforms, this clamps the result, but does not set errno, for 
values beyond int but still within the range of long.  Which is 
different than what it does on 32-bit platforms.  Gross.  The testsuite 
is missing coverage of this, which ideally would be the same behavior 
(setting errno=ERANGE) on both platforms.

> +    } else {
> +        *result = lresult;
> +    }
> +    return check_strtox_error(nptr, ep, endptr, errno);
> +}
> +
> +/**
> + * Convert string @nptr to an unsigned int, and store it in @result.

> + * Note that a number with a leading minus sign gets converted without
> + * the minus sign, checked for overflow (see above), then negated (in
> + * @result's type).  This is exactly how strtoul() works.
> + */
> +int qemu_strtoui(const char *nptr, const char **endptr, int base,
> +                 unsigned int *result)
> +{

> +
> +    errno = 0;
> +    lresult = strtol(nptr, &ep, base);
> +    /* Windows returns 1 for negative out-of-range values.  */
> +    if (errno == ERANGE) {
> +        *result = -1;
> +    } else {
> +        if (lresult > UINT_MAX) {
> +            *result = UINT_MAX;
> +        } else if (lresult < INT_MIN) {
> +            *result = UINT_MAX;

Again, an unfortunate difference between 32- and 64-bit platforms on 
whether errno is set.


-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org