qemu-s390x
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [qemu-s390x] [Qemu-devel] [PATCH v4 04/10] s390-ccw: update libc


From: Eric Blake
Subject: Re: [qemu-s390x] [Qemu-devel] [PATCH v4 04/10] s390-ccw: update libc
Date: Tue, 23 Jan 2018 13:23:38 -0600
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.2

On 01/23/2018 12:26 PM, Collin L. Walling wrote:
> Moved:
>   memcmp from bootmap.h to libc.h (renamed from _memcmp)
>   strlen from sclp.c to libc.h (renamed from _strlen)
> 
> Added C standard functions:
>   atoi
>   isdigit
> 
> Added non C-standard function:
>   itostr
> 
> Signed-off-by: Collin L. Walling <address@hidden>
> Acked-by: Christian Borntraeger <address@hidden>
> Reviewed-by: Janosch Frank <address@hidden>
> ---

> +/**
> + * atoi:
> + * @str: the string to be converted.
> + *
> + * Given a string @str, convert it to an integer. Leading whitespace is
> + * ignored. The first character (after any whitespace) is checked for the
> + * negative sign. Any other non-numerical value will terminate the
> + * conversion.
> + *
> + * Returns: an integer converted from the string @str.
> + */
> +int atoi(const char *str)
> +{
> +    int val = 0;
> +    int sign = 1;
> +
> +    if (!str || !str[0]) {
> +        return 0;
> +    }
> +
> +    while (*str == ' ') {
> +        str++;
> +    }

That's not "any whitespace", but only spaces.  A fully compliant
implementation would be checking isspace(), but I don't expect you to
implement that; at a minimum, also checking '\t' would get you closer
(but not all the way to) compliance.


> +static char *_itostr(int num, char *str, size_t len)
> +{
> +    int num_idx = 0;
> +    int tmp = num;
> +    char sign = 0;
> +
> +    if (!str) {
> +        return NULL;
> +    }
> +
> +    /* Get index to ones place */
> +    while ((tmp /= 10) != 0) {
> +        num_idx++;
> +    }
> +
> +    if (num < 0) {
> +        num *= -1;
> +        sign = 1;
> +    }

If num == INT_MIN, then num is still negative at this point...

> +
> +    /* Check if we have enough space for num, sign, and null */
> +    if (len <= num_idx + sign + 1) {
> +        return NULL;
> +    }
> +
> +    str[num_idx + sign + 1] = '\0';
> +
> +    /* Convert int to string */
> +    while (num_idx >= 0) {
> +        str[num_idx + sign] = num % 10 + '0';

...which breaks this.

Either make it work, or document the corner case as unsupported.

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

Attachment: signature.asc
Description: OpenPGP digital signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]