Re: [qemu-s390x] [PATCH v7 04/12] s390-ccw: update libc

[Thomas Huth]

On 16.02.2018 23:07, Collin L. Walling wrote:
[...]
> +/**
> + * uitoa:
> + * @num: an integer (base 10) to be converted.
> + * @str: a pointer to a string to store the conversion.
> + * @len: the length of the passed string.
> + *
> + * Given an integer @num, convert it to a string. The string @str must be
> + * allocated beforehand. The resulting string will be null terminated and
> + * returned. This function only handles numbers between 0 and UINT64_MAX
> + * inclusive.
> + *
> + * Returns: the string @str of the converted integer @num
> + */
> +char *uitoa(uint64_t num, char *str, size_t len)
> +{
> +    size_t num_idx = 0;
> +    uint64_t tmp = num;
> +
> +    IPL_assert(str != NULL, "uitoa: no space allocated to store string");
> +
> +    /* Get index to ones place */
> +    while ((tmp /= 10) != 0) {
> +        num_idx++;
> +    }
> +
> +    /* Check if we have enough space for num and null */
> +    IPL_assert(len > num_idx, "uitoa: array too small for conversion");

Well, in v5 of this patch you've had "len >= num_idx + 1" where we
agreed that it was wrong. Now you have "len > num_idx" which is pretty
much the same. WTF?
I still think you need "len > num_idx + 1" here to properly take the
trailing NUL-byte into account properly. Please fix it!

 Thomas