Re: [Qemu-devel] [PATCH] monitor: fix parsing of big int

[Fam Zheng]

On Thu, 08/01 10:00, Luiz Capitulino wrote:
> On Thu, 01 Aug 2013 07:52:17 -0600
> Eric Blake <address@hidden> wrote:
> 
> > On 08/01/2013 12:31 AM, Fam Zheng wrote:
> > > Fix it by calling strtoll instead, which will report ERANGE as expected.
> > > 
> > >     (HMP) block_set_io_throttle ide0-hd0 999999999999999999 0 0 0 0 0
> > >     (HMP) block_set_io_throttle ide0-hd0 9999999999999999999 0 0 0 0 0
> > >     number too large
> > >     (HMP) block_set_io_throttle ide0-hd0 99999999999999999999 0 0 0 0 0
> > >     number too large
> > 
> > Your change causes this error message:
> > (HMP) block_set_io_throttle ide0-hd0 -99999999999999999999 0 0 0 0 0
> > number too large
> > 
> > Does the "too large" mean in magnitude (correct message) or in value
> > (misleading message, as any negative number is smaller in value than our
> > minimum of 0)?
> > 
> > > 
> > > Signed-off-by: Fam Zheng <address@hidden>
> > > ---
> > >  monitor.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/monitor.c b/monitor.c
> > > index 5dc0aa9..7bfb469 100644
> > > --- a/monitor.c
> > > +++ b/monitor.c
> > > @@ -3286,7 +3286,7 @@ static int64_t expr_unary(Monitor *mon)
> > >          break;
> > >      default:
> > >          errno = 0;
> > > -        n = strtoull(pch, &p, 0);
> > > +        n = strtoll(pch, &p, 0);
> > 
> > I'm worried that this will break callers that treat their argument as
> > unsigned, and where the full range of unsigned input was desirable.  At
> > this point, it's probably safer to do a case-by-case analysis of all
> > callers that use expr_unary() to decide which callers must reject
> > negative values, instead of making the parser reject numbers that it
> > previously accepted, thus changing the behavior of callers that treated
> > the result as unsigned.
> > 
> 
> Fam, what motivated this change? Is anyone entering such big numbers
> for block_set_io_throttle?

It's for:

https://bugzilla.redhat.com/show_bug.cgi?id=988701#c1

In practice I don't think anyone entering such big numbers, it's corner
case. But as this seems an expr_unary() bug for me (input is big int,
ret value is negative), I try to fix it.

-- 
Fam