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

[Luiz Capitulino <lcapitulino@redhat.com>]

On Thu, 01 Aug 2013 07:52:17 -0600
Eric Blake <eblake@redhat.com> 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 <famz@redhat.com>
> > ---
> >  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?