Move the minimum memory check from the environment parsing to set_memsize, so the limit is actually enforced also when using the API. Adapt the rhbz557655.sh test to the invalid memsize values being rejected now, and add a new test for checking invalid parameters explicitly. --- fish/Makefile.am | 1 + fish/test-invalid-params.sh | 71 ++++++++++++++++++++++++++++ src/handle.c | 13 +++-- tests/regressions/rhbz557655-expected.stdout | 7 ++- tests/regressions/rhbz557655.sh | 8 ++-- 5 files changed, 88 insertions(+), 12 deletions(-) create mode 100755 fish/test-invalid-params.sh diff --git a/fish/Makefile.am b/fish/Makefile.am index ad984ec..9ce1daf 100644 --- a/fish/Makefile.am +++ b/fish/Makefile.am @@ -272,6 +272,7 @@ TESTS = \ test-d.sh \ test-escapes.sh \ test-events.sh \ + test-invalid-params.sh \ test-tilde.sh if ENABLE_APPLIANCE diff --git a/fish/test-invalid-params.sh b/fish/test-invalid-params.sh new file mode 100755 index 0000000..60da094 --- /dev/null +++ b/fish/test-invalid-params.sh @@ -0,0 +1,71 @@ +#!/bin/bash - +# libguestfs +# Copyright (C) 2014 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., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + +# Test passing invalid parameters for memory size, smp, etc when setting up +# the appliance. + +set -e + +# Memory size +output=$( +$VG ./guestfish <<EOF +set-memsize 400 +-set-memsize 0 +-set-memsize 100 +-set-memsize -500 +-set-memsize 0x10 +-set-memsize 010 +-set-memsize -1073741824 +get-memsize +EOF +) +if [ "$output" != "400" ]; then + echo "$0: error: output of guestfish after memsize commands did not match expected output" + echo "$output" + exit 1 +fi + +# smp +output=$( +$VG ./guestfish <<EOF +set-smp 2 +-set-smp 0 +-set-smp 300 +-set-smp -2 +get-smp +EOF +) +if [ "$output" != "2" ]; then + echo "$0: error: output of guestfish after smp commands did not match expected output" + echo "$output" + exit 1 +fi + +# Backend +output=$( +$VG ./guestfish <<EOF +set-backend direct +-set-backend backend-which-does-not-exist +get-backend +EOF +) +if [ "$output" != "direct" ]; then + echo "$0: error: output of guestfish after backend commands did not match expected output" + echo "$output" + exit 1 +fi diff --git a/src/handle.c b/src/handle.c index 719bbcd..0200528 100644 --- a/src/handle.c +++ b/src/handle.c @@ -226,11 +226,14 @@ parse_environment (guestfs_h *g, str = do_getenv (data, "LIBGUESTFS_MEMSIZE"); if (str) { - if (sscanf (str, "%d", &memsize) != 1 || memsize < MIN_MEMSIZE) { - error (g, _("non-numeric or too small value for LIBGUESTFS_MEMSIZE")); + if (sscanf (str, "%d", &memsize) != 1) { + error (g, _("non-numeric value for LIBGUESTFS_MEMSIZE")); + return -1; + } + if (guestfs_set_memsize (g, memsize) == -1) { + /* set_memsize produces an error message already. */ return -1; } - guestfs_set_memsize (g, memsize); } str = do_getenv (data, "LIBGUESTFS_BACKEND"); @@ -552,6 +555,10 @@ guestfs__get_append (guestfs_h *g) int guestfs__set_memsize (guestfs_h *g, int memsize) { + if (memsize < MIN_MEMSIZE) { + error (g, _("too small value for memsize (must be at least %d)"), MIN_MEMSIZE); + return -1; + } g->memsize = memsize; return 0; } diff --git a/tests/regressions/rhbz557655-expected.stdout b/tests/regressions/rhbz557655-expected.stdout index 80bc8bc..abc08e0 100644 --- a/tests/regressions/rhbz557655-expected.stdout +++ b/tests/regressions/rhbz557655-expected.stdout @@ -1,7 +1,6 @@ -0 -16 -8 --1073741824 +500 +512 +4096 1073741823 1234 1234 diff --git a/tests/regressions/rhbz557655.sh b/tests/regressions/rhbz557655.sh index 750263b..87eb279 100755 --- a/tests/regressions/rhbz557655.sh +++ b/tests/regressions/rhbz557655.sh @@ -27,13 +27,11 @@ export LANG=C ../../fish/guestfish >> rhbz557655.out 2>> rhbz557655.err <<EOF # set-memsize is just a convenient non-daemon function that # takes a single integer argument. -set-memsize 0 +set-memsize 500 get-memsize -set-memsize 0x10 +set-memsize 0x200 get-memsize -set-memsize 010 -get-memsize -set-memsize -1073741824 +set-memsize 010000 get-memsize set-memsize 1073741823 get-memsize

Use %d to parse them as int (since the variables for them as int) instead of %u, even if they both need to be at least > 0; the library will check for the validity of them anyway. --- rescue/rescue.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rescue/rescue.c b/rescue/rescue.c index dc56d4b..c73db55 100644 --- a/rescue/rescue.c +++ b/rescue/rescue.c @@ -157,7 +157,7 @@ main (int argc, char *argv[]) else format = optarg; } else if (STREQ (long_options[option_index].name, "smp")) { - if (sscanf (optarg, "%u", &smp) != 1) { + if (sscanf (optarg, "%d", &smp) != 1) { fprintf (stderr, _("%s: could not parse --smp parameter '%s'\n"), program_name, optarg); exit (EXIT_FAILURE); @@ -208,7 +208,7 @@ main (int argc, char *argv[]) break; case 'm': - if (sscanf (optarg, "%u", &memsize) != 1) { + if (sscanf (optarg, "%d", &memsize) != 1) { fprintf (stderr, _("%s: could not parse memory size '%s'\n"), program_name, optarg); exit (EXIT_FAILURE);