[Libguestfs] [PATCH] guestfish: Use xstrtol to parse integers (RHBZ#557655).

Richard W.M. Jones rjones at redhat.com
Fri Jan 22 11:25:46 UTC 2010


See also:

https://bugzilla.redhat.com/show_bug.cgi?id=557655

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-p2v converts physical machines to virtual machines.  Boot with a
live CD or over the network (PXE) and turn machines into Xen guests.
http://et.redhat.com/~rjones/virt-p2v
-------------- next part --------------
>From 1ddc2d465a6b133c15589f7e294c14feaf09b263 Mon Sep 17 00:00:00 2001
From: Richard Jones <rjones at redhat.com>
Date: Fri, 22 Jan 2010 11:01:06 +0000
Subject: [PATCH] guestfish: Use xstrtol to parse integers (RHBZ#557655).

Current code uses atoi to parse the generator Int type and
atoll to parse the generator Int64 type.  The problem with the
ato* functions is that they don't cope with errors very well,
and they cannot parse numbers that begin with 0.. or 0x..
for octal and hexadecimal respectively.

This replaces the atoi call with a call to Gnulib xstrtol.
(atoll cannot be replaced yet).

The generated code looks like this:

  {
    strtol_error xerr;
    long r;

    xerr = xstrtol (argv[0], NULL, 0, &r, "");
    if (xerr != LONGINT_OK) {
      fprintf (stderr,
               _("%s: %s: invalid integer parameter (%s returned %d)\n"),
               cmd, "memsize", "xstrtol", xerr);
      return -1;
    }
    if (r < -1073741824LL || r > 1073741823LL) {
      fprintf (stderr, _("%s: %s: integer out of range\n"), cmd, "memsize");
      return -1;
    }
    /* The check above should ensure this assignment does not overflow. */
    memsize = r;
  }
---
 .gitignore                          |    1 +
 bootstrap                           |    1 +
 m4/.gitignore                       |    5 +++
 regressions/Makefile.am             |    4 ++-
 regressions/rhbz557655-expected.out |   13 ++++++++
 regressions/rhbz557655.sh           |   53 +++++++++++++++++++++++++++++++++++
 src/generator.ml                    |   33 +++++++++++++++++++++-
 7 files changed, 108 insertions(+), 2 deletions(-)
 create mode 100644 regressions/rhbz557655-expected.out
 create mode 100755 regressions/rhbz557655.sh

diff --git a/.gitignore b/.gitignore
index 7e82123..0efdeab 100644
--- a/.gitignore
+++ b/.gitignore
@@ -205,6 +205,7 @@ python/guestfs.py
 python/guestfs-py.c
 python/guestfs.pyc
 regressions/test1.img
+regressions/test.out
 ruby/bindtests.rb
 ruby/ext/guestfs/extconf.h
 ruby/ext/guestfs/_guestfs.c
diff --git a/bootstrap b/bootstrap
index 9ee006e..cb79016 100755
--- a/bootstrap
+++ b/bootstrap
@@ -79,6 +79,7 @@ strndup
 vasprintf
 vc-list-files
 warnings
+xstrtol
 '
 
 $gnulib_tool			\
diff --git a/m4/.gitignore b/m4/.gitignore
index 9ce838b..4697d2c 100644
--- a/m4/.gitignore
+++ b/m4/.gitignore
@@ -134,3 +134,8 @@ xsize.m4
 /safe-write.m4
 /ssize_t.m4
 /write.m4
+/getopt.m4
+/stat.m4
+/symlink.m4
+/time_h.m4
+/xstrtol.m4
diff --git a/regressions/Makefile.am b/regressions/Makefile.am
index 7ceb0ce..3ce4b8d 100644
--- a/regressions/Makefile.am
+++ b/regressions/Makefile.am
@@ -26,6 +26,7 @@ include $(top_srcdir)/subdir-rules.mk
 TESTS = \
 	rhbz503169c10.sh \
 	rhbz503169c13.sh \
+	rhbz557655.sh \
 	test-cancellation-download-librarycancels.sh \
 	test-cancellation-upload-daemoncancels.sh \
 	test-find0.sh \
@@ -55,4 +56,5 @@ TESTS_ENVIRONMENT = \
 EXTRA_DIST = \
 	$(FAILING_TESTS) \
 	$(SKIPPED_TESTS) \
-	$(TESTS)
+	$(TESTS) \
+	rhbz557655-expected.out
diff --git a/regressions/rhbz557655-expected.out b/regressions/rhbz557655-expected.out
new file mode 100644
index 0000000..2241b21
--- /dev/null
+++ b/regressions/rhbz557655-expected.out
@@ -0,0 +1,13 @@
+0
+16
+8
+-1073741824
+1073741823
+set-memsize: memsize: integer out of range
+set-memsize: memsize: integer out of range
+set-memsize: memsize: integer out of range
+set-memsize: memsize: integer out of range
+set-memsize: memsize: invalid integer parameter (xstrtol returned 4)
+set-memsize: memsize: invalid integer parameter (xstrtol returned 2)
+set-memsize: memsize: invalid integer parameter (xstrtol returned 2)
+set-memsize: memsize: invalid integer parameter (xstrtol returned 2)
diff --git a/regressions/rhbz557655.sh b/regressions/rhbz557655.sh
new file mode 100755
index 0000000..6942c57
--- /dev/null
+++ b/regressions/rhbz557655.sh
@@ -0,0 +1,53 @@
+#!/bin/bash -
+# libguestfs
+# Copyright (C) 2009 Red Hat Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+
+# Regression test for:
+# https://bugzilla.redhat.com/show_bug.cgi?id=557655
+# "guestfish number parsing should not use atoi, should support '0...' for octal and '0x...' for hexadecimal"
+
+set -e
+
+rm -f test.out
+
+export LANG=C
+../fish/guestfish > test.out 2>&1 <<EOF
+# set-memsize is just a convenient non-daemon function that
+# takes a single integer argument.
+set-memsize 0
+get-memsize
+set-memsize 0x10
+get-memsize
+set-memsize 010
+get-memsize
+set-memsize -1073741824
+get-memsize
+set-memsize 1073741823
+get-memsize
+# The following should all provoke error messages.
+-set-memsize -9000000000000000
+-set-memsize 9000000000000000
+-set-memsize 0x900000000000
+-set-memsize 07777770000000000000
+-set-memsize ABC
+-set-memsize 09
+-set-memsize 123K
+-set-memsize 123L
+EOF
+
+diff -u test.out rhbz557655-expected.out
+rm test.out
diff --git a/src/generator.ml b/src/generator.ml
index 4bf4b0f..01d343b 100755
--- a/src/generator.ml
+++ b/src/generator.ml
@@ -6856,6 +6856,7 @@ and generate_fish_cmds () =
   pr "\n";
   pr "#include <guestfs.h>\n";
   pr "#include \"c-ctype.h\"\n";
+  pr "#include \"xstrtol.h\"\n";
   pr "#include \"fish.h\"\n";
   pr "\n";
 
@@ -7067,6 +7068,29 @@ and generate_fish_cmds () =
       pr "    fprintf (stderr, _(\"type 'help %%s' for help on %%s\\n\"), cmd, cmd);\n";
       pr "    return -1;\n";
       pr "  }\n";
+
+      let parse_integer fn fntyp rtyp min max name i =
+        pr "  {\n";
+        pr "    strtol_error xerr;\n";
+        pr "    %s r;\n" fntyp;
+        pr "\n";
+        pr "    xerr = %s (argv[%d], NULL, 0, &r, \"\");\n" fn i;
+        pr "    if (xerr != LONGINT_OK) {\n";
+        pr "      fprintf (stderr,\n";
+        pr "               _(\"%%s: %%s: invalid integer parameter (%%s returned %%d)\\n\"),\n";
+        pr "               cmd, \"%s\", \"%s\", xerr);\n" name fn;
+        pr "      return -1;\n";
+        pr "    }\n";
+        pr "    if (r < %LdLL || r > %LdLL) {\n" min max;
+        pr "      fprintf (stderr, _(\"%%s: %%s: integer out of range\\n\"), cmd, \"%s\");\n"
+          name;
+        pr "      return -1;\n";
+        pr "    }\n";
+        pr "    /* The check above should ensure this assignment does not overflow. */\n";
+        pr "    %s = r;\n" name;
+        pr "  }\n";
+      in
+
       iteri (
         fun i ->
           function
@@ -7092,8 +7116,15 @@ and generate_fish_cmds () =
           | Bool name ->
               pr "  %s = is_true (argv[%d]) ? 1 : 0;\n" name i
           | Int name ->
-              pr "  %s = atoi (argv[%d]);\n" name i
+              (* The Int type (see definition above) is defined as a
+               * signed "smallish" int, where that really means
+               * anything that can fit in an OCaml int on a 32 bit
+               * platform (which is 31 bits).  Hence:
+               *)
+              let min = -1073741824L and max = 1073741823L in
+              parse_integer "xstrtol" "long" "int" min max name i
           | Int64 name ->
+              (* parse_integer "xstrtoll" -- no such function in gnulib yet *)
               pr "  %s = atoll (argv[%d]);\n" name i
       ) (snd style);
 
-- 
1.6.5.2



More information about the Libguestfs mailing list