On Thu, 01/14 16:50, Max Reitz wrote: > On 14.01.2016 16:46, Max Reitz wrote: > > On 14.01.2016 05:08, Fam Zheng wrote: > >> The implicit casting from unsigned int to double changes negative values > >> into large positive numbers and accepts them. We should instead print > >> an error. > >> > >> Check the number range so this case is caught and reported. > >> > >> Signed-off-by: Fam Zheng <[email protected]> > >> --- > >> blockdev.c | 3 ++- > >> include/qemu/throttle.h | 2 ++ > >> util/throttle.c | 16 ++++++---------- > >> 3 files changed, 10 insertions(+), 11 deletions(-) > > > > Reviewed-by: Max Reitz <[email protected]> > > > >> diff --git a/blockdev.c b/blockdev.c > >> index 2df0c6d..1afef87 100644 > >> --- a/blockdev.c > >> +++ b/blockdev.c > >> @@ -348,7 +348,8 @@ static bool check_throttle_config(ThrottleConfig *cfg, > >> Error **errp) > >> } > >> > >> if (!throttle_is_valid(cfg)) { > >> - error_setg(errp, "bps/iops/maxs values must be 0 or greater"); > >> + error_setg(errp, "bps/iops/maxs values must be within [0, %" > >> PRId64 > > Pre-existing, but you might want to fix this to "bps/iops/max" while > touching this line.
Makes sense, I'll apply your rev-by and fix this in v4. Fam > > Max > > >> + ")", (int64_t)THROTTLE_VALUE_MAX); > > > > I personally would have liked a simpler %lli and no cast, but I can see > > why you want an explicit int64_t here. > > > >> return false; > >> } > >> > >> diff --git a/include/qemu/throttle.h b/include/qemu/throttle.h > >> index 12faaad..d0c98ed 100644 > >> --- a/include/qemu/throttle.h > >> +++ b/include/qemu/throttle.h > >> @@ -29,6 +29,8 @@ > >> #include "qemu-common.h" > >> #include "qemu/timer.h" > >> > >> +#define THROTTLE_VALUE_MAX 1000000000000000LL > > > > <pedantic> > > But then you could use UINT64_C(1000000000000000) here. > > </pedantic> > > > >
