Re: [Qemu-devel] [PATCH 1/8 v6] cutils: unsigned int parsing functions

[Eric Blake]

On 01/18/2013 12:41 PM, Eduardo Habkost wrote:
> There are lots of duplicate parsing code using strto*() in QEMU, and
> most of that code is broken in one way or another. Even the visitors
> code have duplicate integer parsing code[1]. This introduces functions
> to help parsing unsigned int values: parse_uint() and parse_uint_full().
> 

> Changes v6:
>  - Return -ERANGE (and set *endptr beyond all digits) for negative
>    numbers
>    Suggested-by: Markus Armbruster <address@hidden>
>  - Clarify that function will consume all digits even in case of
>    overflow/underflow
>  - Doesn't define *v as undefined in case parse_uint_full() finds extra
>    characters. Explicitly document it as set to 0.

Looks sane to me, except for the introduction of one typo (please, can
whoever queues up the PULL request just fix that, without needing to see
a v7)?

> 
> 6 version of a simple integer parsing function. This is getting funny.
> :-)

Yeah, but just think how much easier parse_int() is going to be :)

> ---
>  include/qemu-common.h |   4 +
>  tests/Makefile        |   3 +
>  tests/test-cutils.c   | 251 
> ++++++++++++++++++++++++++++++++++++++++++++++++++
>  util/cutils.c         |  99 ++++++++++++++++++++
>  4 files changed, 357 insertions(+)
>  create mode 100644 tests/test-cutils.c

Reviewed-by: Eric Blake <address@hidden>

> + *
> + * Parsed syntax is like strtoull()'s: arbitrary whitespace, a single 
> optional
> + * '+' or '-', an optional "0x"if @base is 0 or 16, one or more digits.

s/"0x"if/"0x" if/

> +
> +/**
> + * parse_uint_full:
> + *
> + * @s: String to parse
> + * @value: Destination for parsed integer value
> + * @base: integer base, between 2 and 36 inclusive, or 0
> + *
> + * Parse unsigned integer from entire string
> + *
> + * Have the same behavior of parse_uint(), but with an additional check
> + * for additional data after the parsed number. If extra characters are 
> present
> + * after the parsed number, the function will return -EINVAL, and 
> address@hidden will
> + * be set to 0.
> + */
> +int parse_uint_full(const char *s, unsigned long long *value, int base)

[Side note - in libvirt, we have just one function instead of two; you
get the parse_uint_full semantics by passing NULL for endptr to the
parse_uint counterpart.  But I'm fine with your approach of mandating
endptr to be a non-NULL pointer to parse_uint, and having a second
function for the case where no end characters are allowed.]

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature