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
