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

[Eduardo Habkost]

On Wed, Jan 16, 2013 at 10:50:18AM -0700, Eric Blake wrote:
> On 01/16/2013 10:33 AM, Eduardo Habkost wrote:
> > On Wed, Jan 16, 2013 at 10:10:42AM -0700, Eric Blake wrote:
> >> On 01/16/2013 08:24 AM, 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().
> >>>
> >>
> >> Your test case lacks test of octal or hexadecimal input strings; is that
> >> worth adding?
> > 
> > I believe I trust strtoll() enough to not require tests for those cases.
> 
> But see my comments in 8/8 about whether it makes sense to add a 'base'
> parameter to let the caller choose whether they want to allow octal or
> require strict decimal parsing.

In that case, I will add at least a test cases to check if "010" gives
the expected result depending on the 'base' parameter.

> 
> >>> +        r =  -errno;
> >>
> >> Why two spaces?
> > 
> > Typo. I will send a fixed version (keeping your Reviewed-by, if you
> > don't mind).
> 
> No problem - I wouldn't have left a reviewed-by if I didn't think the
> changes couldn't be trivially fixed.  On the other hand, you may have a
> non-trivial fix in the form of adding a base parameter...

I was planning to keep the reviewed-by line only if fixing the trivial
whitespace issue above. I won't keep it when adding the 'base'
parameter, don't worry. :-)

-- 
Eduardo