diff mbox series

[1/2] qemu-img: Use qemu_strtoul() rather than raw strtoul()

Message ID 1486744104-15590-2-git-send-email-peter.maydell@linaro.org
State Superseded
Headers show
Series qemu-img: fix coverity nits | expand

Commit Message

Peter Maydell Feb. 10, 2017, 4:28 p.m. UTC 
Some of the argument parsing in qemu-img uses strtoul() to parse
integer arguments.  This is tricky to get correct and in fact the
code does not get it right, because it assigns the result of
strtoul() to an 'int' variable and then tries to check for > INT_MAX.
Coverity correctly complains that the comparison is always false.

Rewrite to use qemu_strtoul(), which has a saner convention for
reporting conversion failures.

(Fixes CID 1356421, CID 1356422, CID 1356423.)

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

---
 qemu-img.c | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

-- 
2.7.4

Comments

Philippe Mathieu-Daudé Feb. 11, 2017, 3:56 a.m. UTC | #1 
On 02/10/2017 01:28 PM, Peter Maydell wrote:
> Some of the argument parsing in qemu-img uses strtoul() to parse

> integer arguments.  This is tricky to get correct and in fact the

> code does not get it right, because it assigns the result of

> strtoul() to an 'int' variable and then tries to check for > INT_MAX.

> Coverity correctly complains that the comparison is always false.

>

> Rewrite to use qemu_strtoul(), which has a saner convention for

> reporting conversion failures.

>

> (Fixes CID 1356421, CID 1356422, CID 1356423.)

>

> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>


Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>


> ---

>  qemu-img.c | 32 ++++++++++++++++----------------

>  1 file changed, 16 insertions(+), 16 deletions(-)

>

> diff --git a/qemu-img.c b/qemu-img.c

> index 74e3362..aa71588 100644

> --- a/qemu-img.c

> +++ b/qemu-img.c

> @@ -3621,24 +3621,24 @@ static int img_bench(int argc, char **argv)

>              break;

>          case 'c':

>          {

> -            char *end;

> -            errno = 0;

> -            count = strtoul(optarg, &end, 0);

> -            if (errno || *end || count > INT_MAX) {

> +            unsigned long res;

> +

> +            if (qemu_strtoul(optarg, NULL, 0, &res) < 0 || res > INT_MAX) {

>                  error_report("Invalid request count specified");

>                  return 1;

>              }

> +            count = res;

>              break;

>          }

>          case 'd':

>          {

> -            char *end;

> -            errno = 0;

> -            depth = strtoul(optarg, &end, 0);

> -            if (errno || *end || depth > INT_MAX) {

> +            unsigned long res;

> +

> +            if (qemu_strtoul(optarg, NULL, 0, &res) < 0 || res > INT_MAX) {

>                  error_report("Invalid queue depth specified");

>                  return 1;

>              }

> +            depth = res;

>              break;

>          }

>          case 'f':

> @@ -3705,24 +3705,24 @@ static int img_bench(int argc, char **argv)

>              break;

>          case OPTION_PATTERN:

>          {

> -            char *end;

> -            errno = 0;

> -            pattern = strtoul(optarg, &end, 0);

> -            if (errno || *end || pattern > 0xff) {

> +            unsigned long res;

> +

> +            if (qemu_strtoul(optarg, NULL, 0, &res) < 0 || res > 0xff) {

>                  error_report("Invalid pattern byte specified");

>                  return 1;

>              }

> +            pattern = res;

>              break;

>          }

>          case OPTION_FLUSH_INTERVAL:

>          {

> -            char *end;

> -            errno = 0;

> -            flush_interval = strtoul(optarg, &end, 0);

> -            if (errno || *end || flush_interval > INT_MAX) {

> +            unsigned long res;

> +

> +            if (qemu_strtoul(optarg, NULL, 0, &res) < 0 || res > INT_MAX) {

>                  error_report("Invalid flush interval specified");

>                  return 1;

>              }

> +            flush_interval = res;

>              break;

>          }

>          case OPTION_NO_DRAIN:

>
diff mbox series

Patch

diff --git a/qemu-img.c b/qemu-img.c
index 74e3362..aa71588 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -3621,24 +3621,24 @@  static int img_bench(int argc, char **argv)
             break;
         case 'c':
         {
-            char *end;
-            errno = 0;
-            count = strtoul(optarg, &end, 0);
-            if (errno || *end || count > INT_MAX) {
+            unsigned long res;
+
+            if (qemu_strtoul(optarg, NULL, 0, &res) < 0 || res > INT_MAX) {
                 error_report("Invalid request count specified");
                 return 1;
             }
+            count = res;
             break;
         }
         case 'd':
         {
-            char *end;
-            errno = 0;
-            depth = strtoul(optarg, &end, 0);
-            if (errno || *end || depth > INT_MAX) {
+            unsigned long res;
+
+            if (qemu_strtoul(optarg, NULL, 0, &res) < 0 || res > INT_MAX) {
                 error_report("Invalid queue depth specified");
                 return 1;
             }
+            depth = res;
             break;
         }
         case 'f':
@@ -3705,24 +3705,24 @@  static int img_bench(int argc, char **argv)
             break;
         case OPTION_PATTERN:
         {
-            char *end;
-            errno = 0;
-            pattern = strtoul(optarg, &end, 0);
-            if (errno || *end || pattern > 0xff) {
+            unsigned long res;
+
+            if (qemu_strtoul(optarg, NULL, 0, &res) < 0 || res > 0xff) {
                 error_report("Invalid pattern byte specified");
                 return 1;
             }
+            pattern = res;
             break;
         }
         case OPTION_FLUSH_INTERVAL:
         {
-            char *end;
-            errno = 0;
-            flush_interval = strtoul(optarg, &end, 0);
-            if (errno || *end || flush_interval > INT_MAX) {
+            unsigned long res;
+
+            if (qemu_strtoul(optarg, NULL, 0, &res) < 0 || res > INT_MAX) {
                 error_report("Invalid flush interval specified");
                 return 1;
             }
+            flush_interval = res;
             break;
         }
         case OPTION_NO_DRAIN: