[PATCH 0/3] Allow environment variables to have boolean values.
https://bugzilla.redhat.com/show_bug.cgi?id=1175196 Currently if you write something like LIBGUESTFS_DEBUG=0 or LIBGUESTFS_DEBUG=true then it doesn't do what you probably expect. This patch series fixes that. Rich.
The 'is_true' function can be useful elsewhere, not just for parsing guestfish command parameters. This is just code motion. --- fish/fish.c | 26 -------------------------- generator/fish.ml | 21 ++++++++++++++++----- src/guestfs-internal-frontend.h | 1 + src/utils.c | 23 +++++++++++++++++++++++ 4 files changed, 40 insertions(+), 31 deletions(-) diff --git a/fish/fish.c b/fish/fish.c index 20511f8..d055e37 100644 --- a/fish/fish.c +++ b/fish/fish.c @@ -1302,32 +1302,6 @@ print_table (char *const *argv) printf ("%s: %s\n", argv[i], argv[i+1]); } -int -is_true (const char *str) -{ - /* Similar to Tcl_GetBoolean. */ - - if (STREQ (str, "1") || - STRCASEEQ (str, "true") || - STRCASEEQ (str, "t") || - STRCASEEQ (str, "yes") || - STRCASEEQ (str, "y") || - STRCASEEQ (str, "on")) - return 1; - - if (STREQ (str, "0") || - STRCASEEQ (str, "false") || - STRCASEEQ (str, "f") || - STRCASEEQ (str, "no") || - STRCASEEQ (str, "n") || - STRCASEEQ (str, "off")) - return 0; - - fprintf (stderr, _("%s: '%s': invalid boolean value, use 'true' or 'false'\n"), - program_name, str); - return -1; -} - /* Free strings from a non-NULL terminated char** */ static void free_n_strings (char **str, size_t len) diff --git a/generator/fish.ml b/generator/fish.ml index b609ce9..c7ef303 100644 --- a/generator/fish.ml +++ b/generator/fish.ml @@ -84,12 +84,15 @@ let generate_fish_cmds () = pr "#include <string.h>\n"; pr "#include <inttypes.h>\n"; pr "#include <libintl.h>\n"; + pr "#include <errno.h>\n"; pr "\n"; pr "#include \"c-ctype.h\"\n"; pr "#include \"full-write.h\"\n"; pr "#include \"xstrtol.h\"\n"; pr "\n"; - pr "#include <guestfs.h>\n"; + pr "#include \"guestfs.h\"\n"; + pr "#include \"guestfs-internal-frontend.h\"\n"; + pr "\n"; pr "#include \"fish.h\"\n"; pr "#include \"fish-cmds.h\"\n"; pr "#include \"options.h\"\n"; @@ -469,8 +472,12 @@ Guestfish will prompt for these separately." pr " input_lineno++;\n"; pr " if (%s == NULL) goto out_%s;\n" name name | Bool name -> - pr " switch (is_true (argv[i++])) {\n"; - pr " case -1: goto out_%s;\n" name; + pr " switch (guestfs___is_true (argv[i++])) {\n"; + pr " case -1:\n"; + pr " fprintf (stderr,\n"; + pr " _(\"%%s: '%%s': invalid boolean value, use 'true' or 'false'\\n\"),\n"; + pr " program_name, argv[i-1]);\n"; + pr " goto out_%s;\n" name; pr " case 0: %s = 0; break;\n" name; pr " default: %s = 1;\n" name; pr " }\n" @@ -508,8 +515,12 @@ Guestfish will prompt for these separately." pr "if (STRPREFIX (argv[i], \"%s:\")) {\n" n; (match argt with | OBool n -> - pr " switch (is_true (&argv[i][%d])) {\n" (len+1); - pr " case -1: goto out;\n"; + pr " switch (guestfs___is_true (&argv[i][%d])) {\n" (len+1); + pr " case -1:\n"; + pr " fprintf (stderr,\n"; + pr " _(\"%%s: '%%s': invalid boolean value, use 'true' or 'false'\\n\"),\n"; + pr " program_name, &argv[i][%d]);\n" (len+1); + pr " goto out;\n"; pr " case 0: optargs_s.%s = 0; break;\n" n; pr " default: optargs_s.%s = 1;\n" n; pr " }\n" diff --git a/src/guestfs-internal-frontend.h b/src/guestfs-internal-frontend.h index cddf719..57aeeda 100644 --- a/src/guestfs-internal-frontend.h +++ b/src/guestfs-internal-frontend.h @@ -105,6 +105,7 @@ extern char **guestfs___split_string (char sep, const char *); extern char *guestfs___exit_status_to_string (int status, const char *cmd_name, char *buffer, size_t buflen); extern int guestfs___random_string (char *ret, size_t len); extern char *guestfs___drive_name (size_t index, char *ret); +extern int guestfs___is_true (const char *str); /* These functions are used internally by the CLEANUP_* macros. * Don't call them directly. diff --git a/src/utils.c b/src/utils.c index be7f643..11c6953 100644 --- a/src/utils.c +++ b/src/utils.c @@ -270,3 +270,26 @@ guestfs___drive_name (size_t index, char *ret) *ret = '\0'; return ret; } + +/* Similar to Tcl_GetBoolean. */ +int +guestfs___is_true (const char *str) +{ + if (STREQ (str, "1") || + STRCASEEQ (str, "true") || + STRCASEEQ (str, "t") || + STRCASEEQ (str, "yes") || + STRCASEEQ (str, "y") || + STRCASEEQ (str, "on")) + return 1; + + if (STREQ (str, "0") || + STRCASEEQ (str, "false") || + STRCASEEQ (str, "f") || + STRCASEEQ (str, "no") || + STRCASEEQ (str, "n") || + STRCASEEQ (str, "off")) + return 0; + + return -1; +} -- 2.1.0
You can now use LIBGUESTFS_DEBUG=true (etc.) You can disable debugging/tracing by setting LIBGUESTFS_DEBUG=0 (etc.) --- fuse/test-guestmount-fd.c | 2 +- src/handle.c | 30 ++++++++++++++++++++------- tests/charsets/test-charset-fidelity.c | 4 ++-- tests/mount-local/test-parallel-mount-local.c | 2 +- tests/parallel/Makefile.am | 1 + tests/parallel/test-parallel.c | 2 +- tests/regressions/Makefile.am | 1 + tests/regressions/rhbz914931.c | 2 +- 8 files changed, 30 insertions(+), 14 deletions(-) diff --git a/fuse/test-guestmount-fd.c b/fuse/test-guestmount-fd.c index 00eab0c..2b3ce5b 100644 --- a/fuse/test-guestmount-fd.c +++ b/fuse/test-guestmount-fd.c @@ -50,7 +50,7 @@ main (int argc, char *argv[]) /* Allow the test to be skipped. */ skip = getenv ("SKIP_TEST_GUESTMOUNT_FD"); - if (skip && STREQ (skip, "1")) { + if (skip && guestfs___is_true (skip) > 0) { fprintf (stderr, "%s: test skipped because environment variable set.\n", program_name); exit (77); diff --git a/src/handle.c b/src/handle.c index 337c375..efda02f 100644 --- a/src/handle.c +++ b/src/handle.c @@ -176,7 +176,7 @@ parse_environment (guestfs_h *g, char *(*do_getenv) (const void *data, const char *), const void *data) { - int memsize; + int memsize, b; char *str; /* Don't bother checking the return values of functions @@ -184,12 +184,24 @@ parse_environment (guestfs_h *g, */ str = do_getenv (data, "LIBGUESTFS_TRACE"); - if (str != NULL && STREQ (str, "1")) - guestfs_set_trace (g, 1); + if (str) { + b = guestfs___is_true (str); + if (b == -1) { + error (g, _("%s=%s: non-boolean value"), "LIBGUESTFS_TRACE", str); + return -1; + } + guestfs_set_trace (g, b); + } str = do_getenv (data, "LIBGUESTFS_DEBUG"); - if (str != NULL && STREQ (str, "1")) - guestfs_set_verbose (g, 1); + if (str) { + b = guestfs___is_true (str); + if (b == -1) { + error (g, _("%s=%s: non-boolean value"), "LIBGUESTFS_TRACE", str); + return -1; + } + guestfs_set_verbose (g, b); + } str = do_getenv (data, "LIBGUESTFS_TMPDIR"); if (str && STRNEQ (str, "")) { @@ -822,6 +834,7 @@ int guestfs___get_backend_setting_bool (guestfs_h *g, const char *name) { CLEANUP_FREE char *value = NULL; + int b; guestfs_push_error_handler (g, NULL, NULL); value = guestfs_get_backend_setting (g, name); @@ -833,10 +846,11 @@ guestfs___get_backend_setting_bool (guestfs_h *g, const char *name) if (value == NULL) return -1; - if (STREQ (value, "1")) - return 1; + b = guestfs___is_true (value); + if (b == -1) + return -1; - return 0; + return b; } int diff --git a/tests/charsets/test-charset-fidelity.c b/tests/charsets/test-charset-fidelity.c index 4b34b0e..7ce7d94 100644 --- a/tests/charsets/test-charset-fidelity.c +++ b/tests/charsets/test-charset-fidelity.c @@ -81,7 +81,7 @@ main (int argc, char *argv[]) /* Allow this test to be skipped. */ str = getenv (ourenvvar); - if (str && STREQ (str, "1")) { + if (str && guestfs___is_true (str) > 0) { printf ("%s: test skipped because environment variable is set.\n", program_name); exit (77); @@ -126,7 +126,7 @@ test_filesystem (guestfs_h *g, const struct filesystem *fs) snprintf (envvar, sizeof envvar, "%s_%s", ourenvvar, fs->fs_name); str = getenv (envvar); - if (str && STREQ (str, "1")) { + if (str && guestfs___is_true (str) > 0) { printf ("skipped test of %s because environment variable is set\n", fs->fs_name); return; diff --git a/tests/mount-local/test-parallel-mount-local.c b/tests/mount-local/test-parallel-mount-local.c index fa6cd79..88ca2d3 100644 --- a/tests/mount-local/test-parallel-mount-local.c +++ b/tests/mount-local/test-parallel-mount-local.c @@ -94,7 +94,7 @@ main (int argc, char *argv[]) /* Allow the test to be skipped by setting an environment variable. */ skip = getenv ("SKIP_TEST_PARALLEL_MOUNT_LOCAL"); - if (skip && STREQ (skip, "1")) { + if (skip && guestfs___is_true (skip) > 0) { fprintf (stderr, "%s: test skipped because environment variable set.\n", program_name); exit (77); diff --git a/tests/parallel/Makefile.am b/tests/parallel/Makefile.am index be63256..9421bfc 100644 --- a/tests/parallel/Makefile.am +++ b/tests/parallel/Makefile.am @@ -34,6 +34,7 @@ test_parallel_CFLAGS = \ -pthread \ $(WARN_CFLAGS) $(WERROR_CFLAGS) test_parallel_LDADD = \ + $(top_builddir)/src/libutils.la \ $(top_builddir)/src/libguestfs.la \ $(top_builddir)/gnulib/lib/libgnu.la diff --git a/tests/parallel/test-parallel.c b/tests/parallel/test-parallel.c index edd87d9..e412143 100644 --- a/tests/parallel/test-parallel.c +++ b/tests/parallel/test-parallel.c @@ -76,7 +76,7 @@ main (int argc, char *argv[]) /* Allow the test to be skipped by setting an environment variable. */ skip = getenv ("SKIP_TEST_PARALLEL"); - if (skip && STREQ (skip, "1")) { + if (skip && guestfs___is_true (skip) > 0) { fprintf (stderr, "%s: test skipped because environment variable set.\n", program_name); exit (77); diff --git a/tests/regressions/Makefile.am b/tests/regressions/Makefile.am index a5e7cfc..de97526 100644 --- a/tests/regressions/Makefile.am +++ b/tests/regressions/Makefile.am @@ -111,6 +111,7 @@ rhbz914931_CFLAGS = \ -pthread \ $(WARN_CFLAGS) $(WERROR_CFLAGS) rhbz914931_LDADD = \ + $(top_builddir)/src/libutils.la \ $(top_builddir)/src/libguestfs.la rhbz1055452_SOURCES = rhbz1055452.c diff --git a/tests/regressions/rhbz914931.c b/tests/regressions/rhbz914931.c index faa3dd2..569e261 100644 --- a/tests/regressions/rhbz914931.c +++ b/tests/regressions/rhbz914931.c @@ -41,7 +41,7 @@ main (int argc, char *argv[]) /* Allow this test to be skipped. */ str = getenv ("SKIP_TEST_RHBZ914931"); - if (str && STREQ (str, "1")) { + if (str && guestfs___is_true (str) > 0) { printf ("%s: test skipped because environment variable is set.\n", program_name); exit (77); -- 2.1.0
--- tests/regressions/Makefile.am | 2 ++ tests/regressions/rhbz1175196.sh | 30 ++++++++++++++++++++++++++++++ 2 files changed, 32 insertions(+) create mode 100755 tests/regressions/rhbz1175196.sh diff --git a/tests/regressions/Makefile.am b/tests/regressions/Makefile.am index de97526..72d26da 100644 --- a/tests/regressions/Makefile.am +++ b/tests/regressions/Makefile.am @@ -42,6 +42,7 @@ EXTRA_DIST = \ rhbz1044014.xml \ rhbz1054761.sh \ rhbz1091803.sh \ + rhbz1175196.sh \ test-noexec-stack.pl TESTS = \ @@ -66,6 +67,7 @@ TESTS = \ rhbz1054761.sh \ rhbz1055452 \ rhbz1091803.sh \ + rhbz1175196.sh \ test-noexec-stack.pl if HAVE_LIBVIRT diff --git a/tests/regressions/rhbz1175196.sh b/tests/regressions/rhbz1175196.sh new file mode 100755 index 0000000..06f6c24 --- /dev/null +++ b/tests/regressions/rhbz1175196.sh @@ -0,0 +1,30 @@ +#!/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. + +# Regression test for: +# https://bugzilla.redhat.com/show_bug.cgi?id=1175196 +# Parse 'LIBGUESTFS_TRACE=0' in the environment. + +set -e +export LANG=C + +if [ "$(guestfish setenv LIBGUESTFS_TRACE 1 : parse-environment : get-trace : setenv LIBGUESTFS_TRACE 0 : parse-environment : get-trace)" != "true +false" ]; then + echo "$0: unexpected output from test" + exit 1 +fi -- 2.1.0
participants (1)
-
Richard W.M. Jones