[Libguestfs] [PATCH 1/2] src: always check value passed to set_memsize

[Richard W.M. Jones]

On Fri, Aug 08, 2014 at 11:29:24AM +0200, Pino Toscano wrote:
> 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

ACK.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
libguestfs lets you edit virtual machines.  Supports shell scripting,
bindings from many languages.  http://libguestfs.org