Re: [Qemu-devel] [PATCH] qdev: Validate hex properties

[Peter Maydell]

On 26 November 2013 15:05, Hannes Reinecke <address@hidden> wrote:
> strtoull() might return '-1' to signify an overflow.
>
> Signed-off-by: Hannes Reinecke <address@hidden>
> ---
>  hw/core/qdev-properties.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
> index dc8ae69..5761295 100644
> --- a/hw/core/qdev-properties.c
> +++ b/hw/core/qdev-properties.c
> @@ -202,6 +202,9 @@ static int parse_hex8(DeviceState *dev, Property *prop, 
> const char *str)
>      if ((*end != '\0') || (end == str)) {
>          return -EINVAL;
>      }
> +    if (*ptr == (uint8_t)-1) {
> +        return -ERANGE;
> +    }

This doesn't look right -- *ptr might legitimately be anything
from 0x00 to 0xff; -1 can be a valid strtoul() return as well as
the error indication.

The right way to check for overflow is to clear errno before the call
to strtoul(), and then check whether it's non-zero after the call;
this is the approach recommended by POSIX.
http://pubs.opengroup.org/onlinepubs/9699919799/functions/strtoul.html

For the hex8 version where we're putting the result in a smaller
sized integer than the type returned by strtoul(), we should
put the result into an unsigned long, check it's in 0..255, and
then stick it in the uint8_t.

thanks
-- PMM