Re: [PATCH v2 1/5] qemu-img: remove check that cvtnum value > MAX_INT

[Eric Blake]

On 5/6/20 4:34 PM, Eyal Moscovici wrote:
> Following commit f46bfdbfc8f95cf65d7818ef68a801e063c40332 (util/cutils: Change
> qemu_strtosz*() from int64_t to uint64_t) which added a similar check to
> cvtnum. As a result there is no need to check it separately outside of cvtnum.
>
> Acked-by: Mark Kanda <address@hidden>
> Signed-off-by: Eyal Moscovici <address@hidden>
> ---
>   qemu-img.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/qemu-img.c b/qemu-img.c
> index 6a4327aaba..116a9c6349 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -4307,7 +4307,7 @@ static int img_bench(int argc, char **argv)
>               int64_t sval;
>   
>               sval = cvtnum(optarg);
> -            if (sval < 0 || sval > INT_MAX) {
> +            if (sval < 0) {
>                   error_report("Invalid buffer size specified");

INT_MAX is smaller than cvtnum's check for INT64_MAX.  This code change 
allows larger buffer sizes, which is probably not a good idea.

>                   return 1;
>               }
> @@ -4320,7 +4320,7 @@ static int img_bench(int argc, char **argv)
>               int64_t sval;
>   
>               sval = cvtnum(optarg);
> -            if (sval < 0 || sval > INT_MAX) {
> +            if (sval < 0) {
>                   error_report("Invalid step size specified");
>                   return 1;
>               }
> @@ -4493,7 +4493,7 @@ static int img_dd_bs(const char *arg,
>   
>       res = cvtnum(arg);
>   
> -    if (res <= 0 || res > INT_MAX) {
> +    if (res <= 0) {
>           error_report("invalid number: '%s'", arg);
>           return 1;
>       }

NACK.

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org