Richard W.M. Jones
2014-Dec-17 14:04 UTC
[Libguestfs] [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.
Richard W.M. Jones
2014-Dec-17 14:04 UTC
[Libguestfs] [PATCH 1/3] fish: Move 'is_true' function to library utilities.
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
Richard W.M. Jones
2014-Dec-17 14:04 UTC
[Libguestfs] [PATCH 2/3] environment: Use guestfs___is_true when parsing various boolean environment variables (RHBZ#1175196).
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
Richard W.M. Jones
2014-Dec-17 14:04 UTC
[Libguestfs] [PATCH 3/3] fish: Add regression test for RHBZ#1175196.
--- 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