5:21 a.m.
Nearly any program that writes to standard output can
benefit from this sort of fix.
Without it, running e.g., ./guestfish --version > /dev/full
would exit successfully, even though it got ENOSPC
when writing to the full device. That means regular
output redirected to a file on a full partition may also
fail to be written, and the error ignored.
Before:
$ guestfish --version > /dev/full
After:
$ ./guestfish --version > /dev/full
/w/co/libguestfs/fish/.libs/lt-guestfish: write error: No space left on device
Obviously, including the absolute program name in the diagnostic is a bug.
It should be "guestfish", and there's already code to perform that
sanitization in the patch below. It fixes everything except the glibc
global variable, program_invocation_name, used by error, called by
atexit+close_stdout. I'll fix that via gnulib's progname module later
today or tomorrow. Note that all other diagnostics (e.g., below) do
use the sanitized name.
The second patch is to make guestfish write --help output
to stdout, not stderr, per convention. Also, since --help
output is pretty long, I find it clearer (e.g., when I misspell
an option) to write a brief diagnostic, like
$ ./guestfish --fjd
guestfish: unrecognized option '--fjd'
Try `guestfish --help' for more information.
[Exit 1]
The third patch is another small improvement: let getopt diagnose failures.
Before, using --mount with no option-argument would print a
misleading diagnostic:
$ ./guestfish -m
guestfish: unexpected command line option 0x3f
...all --help output...
[Exit 1]
Now it prints this:
$ ./guestfish -m
guestfish: option requires an argument -- 'm'
Try `guestfish --help' for more information.
[Exit 1]
------------------------------
The fourth patch makes the Before/After change demonstrated above.
The last patch avoids new cpp macro redefinition warnings.
Now that we're using a few more gnulib modules, they
end up defining _GNU_SOURCE via a definition in config.h,
so the explicit definitons in various *.c files evoke warnings.
$ wc -l 00*
45 0001-build-avoid-some-autoconf-warnings.patch
353 0002-guestfish-write-help-to-stdout-use-gnulib-s-progname.patch
27 0003-guestfish-don-t-try-to-diagnose-getopt-failure.patch
53 0004-guestfish-diagnose-stdout-write-failure.patch
88 0005-build-don-t-define-_GNU_SOURCE-manually.patch
566 total
$ diffstat 00*
b/bootstrap | 1
b/configure.ac | 11 ++--
b/fish/Makefile.am | 3 -
b/fish/destpaths.c | 2
b/fish/fish.c | 125 ++++++++++++++++++++++++++++++++++-------------------
b/src/guestfs.c | 2
bootstrap | 8 ++-
fish/fish.c | 9 +--
8 files changed, 100 insertions(+), 61 deletions(-)
5:21 a.m.
New subject: [PATCH libguestfs 1/5] build: avoid some autoconf warnings
From: Jim Meyering <meyering(a)redhat.com>
* configure.ac: Move gl_EARLY and gl_INIT to be earlier.
---
configure.ac | 10 +++++-----
1 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/configure.ac b/configure.ac
index 5a3379b..2284dac 100644
--- a/configure.ac
+++ b/configure.ac
@@ -31,22 +31,22 @@ AM_SILENT_RULES([yes]) # make --enable-silent-rules the default.
AC_CONFIG_MACRO_DIR([m4])
-AC_PROG_LIBTOOL
-
dnl Split up the version string.
AC_DEFINE([PACKAGE_VERSION_MAJOR],[libguestfs_major],[Major version number])
AC_DEFINE([PACKAGE_VERSION_MINOR],[libguestfs_minor],[Minor version number])
AC_DEFINE([PACKAGE_VERSION_RELEASE],[libguestfs_release],[Release number])
AC_DEFINE([PACKAGE_VERSION_EXTRA],["libguestfs_extra"],[Extra version string])
+gl_EARLY
+gl_INIT
+
+AC_PROG_LIBTOOL
+
dnl Check for basic C environment.
AC_PROG_CC_STDC
AC_PROG_INSTALL
AC_PROG_CPP
-gl_EARLY
-gl_INIT
-
AC_ARG_ENABLE([gcc-warnings],
[AS_HELP_STRING([--enable-gcc-warnings],
[turn on lots of GCC warnings (for developers)])],
--
1.6.4.378.g88f2f
5:21 a.m.
New subject: [PATCH libguestfs 2/5] guestfish: write --help to stdout, use gnulib's progname module
From: Jim Meyering <meyering(a)redhat.com>
* fish/fish.c: Include "progname.h".
(main): Call set_program_name to initialize.
Don't hard-code guestfish everywhere. Use program_name.
(usage): Don't spew all of --help for a mis-typed option.
Split long lines.
---
bootstrap | 1 +
fish/Makefile.am | 3 +-
fish/fish.c | 124 +++++++++++++++++++++++++++++++++++------------------
3 files changed, 85 insertions(+), 43 deletions(-)
diff --git a/bootstrap b/bootstrap
index 76e2216..3fd8811 100755
--- a/bootstrap
+++ b/bootstrap
@@ -56,6 +56,7 @@ gnumakefile
ignore-value
maintainer-makefile
manywarnings
+progname
warnings
vc-list-files
'
diff --git a/fish/Makefile.am b/fish/Makefile.am
index 94bf757..c8ba3ea 100644
--- a/fish/Makefile.am
+++ b/fish/Makefile.am
@@ -52,13 +52,14 @@ guestfish_CFLAGS = \
-I$(top_srcdir)/src -I$(top_builddir)/src \
-I$(top_srcdir)/fish -I$(top_builddir)/fish \
-DGUESTFS_DEFAULT_PATH='"$(libdir)/guestfs"' \
+ -I$(srcdir)/../gnulib/lib -I../gnulib/lib \
$(WARN_CFLAGS) $(WERROR_CFLAGS)
guestfish_LDADD = $(top_builddir)/src/libguestfs.la $(LIBREADLINE)
# Make libguestfs use the convenience library.
noinst_LTLIBRARIES = librc_protocol.la
-guestfish_LDADD += librc_protocol.la
+guestfish_LDADD += librc_protocol.la ../gnulib/lib/libgnu.la
if HAVE_RPCGEN
rc_protocol.c: rc_protocol.x
diff --git a/fish/fish.c b/fish/fish.c
index 3acd450..8ea1c49 100644
--- a/fish/fish.c
+++ b/fish/fish.c
@@ -40,6 +40,7 @@
#include <guestfs.h>
#include "fish.h"
+#include "progname.h"
struct mp {
struct mp *next;
@@ -87,21 +88,25 @@ launch (guestfs_h *_g)
return 0;
}
-static void
-usage (void)
+static void __attribute__((noreturn))
+usage (int status)
{
- fprintf (stderr,
- _("guestfish: guest filesystem shell\n"
- "guestfish lets you edit virtual machine filesystems\n"
+ if (status != EXIT_SUCCESS)
+ fprintf (stderr, _("Try `%s --help' for more information.\n"),
+ program_name);
+ else {
+ fprintf (stdout,
+ _("%s: guest filesystem shell\n"
+ "%s lets you edit virtual machine filesystems\n"
"Copyright (C) 2009 Red Hat Inc.\n"
"Usage:\n"
- " guestfish [--options] cmd [: cmd : cmd ...]\n"
- " guestfish -i libvirt-domain\n"
- " guestfish -i disk-image(s)\n"
+ " %s [--options] cmd [: cmd : cmd ...]\n"
+ " %s -i libvirt-domain\n"
+ " %s -i disk-image(s)\n"
"or for interactive use:\n"
- " guestfish\n"
+ " %s\n"
"or from a shell script:\n"
- " guestfish <<EOF\n"
+ " %s <<EOF\n"
" cmd\n"
" ...\n"
" EOF\n"
@@ -115,24 +120,37 @@ usage (void)
" --listen Listen for remote commands\n"
" -m|--mount dev[:mnt] Mount dev on mnt (if omitted, /)\n"
" -n|--no-sync Don't autosync\n"
- " --remote[=pid] Send commands to remote guestfish\n"
+ " --remote[=pid] Send commands to remote %s\n"
" -r|--ro Mount read-only\n"
" --selinux Enable SELinux support\n"
" -v|--verbose Verbose messages\n"
" -x Echo each command before executing it\n"
" -V|--version Display version and exit\n"
- "For more information, see the manpage guestfish(1).\n"));
+ "For more information, see the manpage %s(1).\n"),
+ program_name, program_name, program_name,
+ program_name, program_name, program_name,
+ program_name, program_name, program_name);
+ }
+ exit (status);
}
int
main (int argc, char *argv[])
{
+ /* Set global program name that is not polluted with libtool artifacts. */
+ set_program_name (argv[0]);
+
+ /* getopt_long uses argv[0], so give it the sanitized name, too. */
+ argv[0] = bad_cast (program_name);
+
+ enum { HELP_OPTION = CHAR_MAX + 1 };
+
static const char *options = "a:Df:h::im:nrv?Vx";
static const struct option long_options[] = {
{ "add", 1, 0, 'a' },
{ "cmd-help", 2, 0, 'h' },
{ "file", 1, 0, 'f' },
- { "help", 0, 0, '?' },
+ { "help", 0, 0, HELP_OPTION },
{ "inspector", 0, 0, 'i' },
{ "listen", 0, 0, 0 },
{ "mount", 1, 0, 'm' },
@@ -197,21 +215,24 @@ main (int argc, char *argv[])
else if (strcmp (long_options[option_index].name, "remote") == 0) {
if (optarg) {
if (sscanf (optarg, "%d", &remote_control) != 1) {
- fprintf (stderr, _("guestfish: --listen=PID: PID was not a number:
%s\n"), optarg);
+ fprintf (stderr, _("%s: --listen=PID: PID was not a number:
%s\n"),
+ program_name, optarg);
exit (1);
}
} else {
p = getenv ("GUESTFISH_PID");
if (!p || sscanf (p, "%d", &remote_control) != 1) {
- fprintf (stderr, _("guestfish: remote: $GUESTFISH_PID must be set to the
PID of the remote process\n"));
+ fprintf (stderr, _("%s: remote: $GUESTFISH_PID must be set"
+ " to the PID of the remote process\n"),
+ program_name);
exit (1);
}
}
} else if (strcmp (long_options[option_index].name, "selinux") == 0) {
guestfs_set_selinux (g, 1);
} else {
- fprintf (stderr, _("guestfish: unknown long option: %s (%d)\n"),
- long_options[option_index].name, option_index);
+ fprintf (stderr, _("%s: unknown long option: %s (%d)\n"),
+ program_name, long_options[option_index].name, option_index);
exit (1);
}
break;
@@ -237,7 +258,8 @@ main (int argc, char *argv[])
case 'f':
if (file) {
- fprintf (stderr, _("guestfish: only one -f parameter can be
given\n"));
+ fprintf (stderr, _("%s: only one -f parameter can be given\n"),
+ program_name);
exit (1);
}
file = optarg;
@@ -287,21 +309,20 @@ main (int argc, char *argv[])
break;
case 'V':
- printf ("guestfish %s\n", PACKAGE_VERSION);
+ printf ("%s %s\n", program_name, PACKAGE_VERSION);
exit (0);
case 'x':
echo_commands = 1;
break;
- case '?':
- usage ();
- exit (0);
+ case HELP_OPTION:
+ usage (0);
default:
- fprintf (stderr, _("guestfish: unexpected command line option 0x%x\n"),
- c);
- exit (1);
+ fprintf (stderr, _("%s: unexpected command line option 0x%x\n"),
+ program_name, c);
+ usage (1);
}
}
@@ -312,11 +333,15 @@ main (int argc, char *argv[])
if (drvs || mps || remote_control_listen || remote_control ||
guestfs_get_selinux (g)) {
- fprintf (stderr, _("guestfish: cannot use -i option with -a, -m, --listen,
--remote or --selinux\n"));
+ fprintf (stderr, _("%s: cannot use -i option with -a, -m,"
+ " --listen, --remote or --selinux\n"),
+ program_name);
exit (1);
}
if (optind >= argc) {
- fprintf (stderr, _("guestfish -i requires a libvirt domain or path(s) to disk
image(s)\n"));
+ fprintf (stderr,
+ _("%s: -i requires a libvirt domain or path(s) to disk
image(s)\n"),
+ program_name);
exit (1);
}
@@ -324,7 +349,9 @@ main (int argc, char *argv[])
while (optind < argc) {
if (strlen (cmd) + strlen (argv[optind]) + strlen (argv[0]) + 60
= sizeof cmd) {
- fprintf (stderr, _("guestfish:
virt-inspector command too long for fixed-size buffer\n"));
+ fprintf (stderr,
+ _("%s: virt-inspector command too long for fixed-size
buffer\n"),
+ program_name);
exit (1);
}
strcat (cmd, " '");
@@ -347,7 +374,7 @@ main (int argc, char *argv[])
if (verbose)
fprintf (stderr,
- "guestfish -i: running virt-inspector command:\n%s\n", cmd);
+ "%s -i: running virt-inspector command:\n%s\n", program_name,
cmd);
r = system (cmd);
if (r == -1) {
@@ -368,17 +395,23 @@ main (int argc, char *argv[])
/* Remote control? */
if (remote_control_listen && remote_control) {
- fprintf (stderr, _("guestfish: cannot use --listen and --remote options at the
same time\n"));
+ fprintf (stderr,
+ _("%s: cannot use --listen and --remote options at the same
time\n"),
+ program_name);
exit (1);
}
if (remote_control_listen) {
if (optind < argc) {
- fprintf (stderr, _("guestfish: extra parameters on the command line with
--listen flag\n"));
+ fprintf (stderr,
+ _("%s: extra parameters on the command line with --listen
flag\n"),
+ program_name);
exit (1);
}
if (file) {
- fprintf (stderr, _("guestfish: cannot use --listen and --file options at the
same time\n"));
+ fprintf (stderr,
+ _("%s: cannot use --listen and --file options at the same
time\n"),
+ program_name);
exit (1);
}
rc_listen ();
@@ -602,12 +635,14 @@ script (int prompt)
p++;
len = strcspn (p, "\"");
if (p[len] == '\0') {
- fprintf (stderr, _("guestfish: unterminated double quote\n"));
+ fprintf (stderr, _("%s: unterminated double quote\n"),
program_name);
if (exit_on_error) exit (1);
goto next_command;
}
if (p[len+1] && (p[len+1] != ' ' && p[len+1] !=
'\t')) {
- fprintf (stderr, _("guestfish: command arguments not separated by
whitespace\n"));
+ fprintf (stderr,
+ _("%s: command arguments not separated by whitespace\n"),
+ program_name);
if (exit_on_error) exit (1);
goto next_command;
}
@@ -617,12 +652,14 @@ script (int prompt)
p++;
len = strcspn (p, "'");
if (p[len] == '\0') {
- fprintf (stderr, _("guestfish: unterminated single quote\n"));
+ fprintf (stderr, _("%s: unterminated single quote\n"),
program_name);
if (exit_on_error) exit (1);
goto next_command;
}
if (p[len+1] && (p[len+1] != ' ' && p[len+1] !=
'\t')) {
- fprintf (stderr, _("guestfish: command arguments not separated by
whitespace\n"));
+ fprintf (stderr,
+ _("%s: command arguments not separated by whitespace\n"),
+ program_name);
if (exit_on_error) exit (1);
goto next_command;
}
@@ -643,12 +680,15 @@ script (int prompt)
pend++;
}
if (c != 0) {
- fprintf (stderr, _("guestfish: unterminated \"[...]\"
sequence\n"));
+ fprintf (stderr,
+ _("%s: unterminated \"[...]\" sequence\n"),
program_name);
if (exit_on_error) exit (1);
goto next_command;
}
if (*pend && (*pend != ' ' && *pend != '\t')) {
- fprintf (stderr, _("guestfish: command arguments not separated by
whitespace\n"));
+ fprintf (stderr,
+ _("%s: command arguments not separated by whitespace\n"),
+ program_name);
if (exit_on_error) exit (1);
goto next_command;
}
@@ -667,8 +707,8 @@ script (int prompt)
} else
pend = &p[len];
} else {
- fprintf (stderr, _("guestfish: internal error parsing string at
'%s'\n"),
- p);
+ fprintf (stderr, _("%s: internal error parsing string at
'%s'\n"),
+ program_name, p);
abort ();
}
@@ -684,7 +724,7 @@ script (int prompt)
}
if (i == sizeof argv / sizeof argv[0]) {
- fprintf (stderr, _("guestfish: too many arguments\n"));
+ fprintf (stderr, _("%s: too many arguments\n"), program_name);
if (exit_on_error) exit (1);
goto next_command;
}
@@ -713,7 +753,7 @@ cmdline (char *argv[], int optind, int argc)
cmd = argv[optind++];
if (strcmp (cmd, ":") == 0) {
- fprintf (stderr, _("guestfish: empty command on command line\n"));
+ fprintf (stderr, _("%s: empty command on command line\n"), program_name);
exit (1);
}
params = &argv[optind];
--
1.6.4.378.g88f2f
5:21 a.m.
New subject: [PATCH libguestfs 3/5] guestfish: don't try to diagnose getopt failure
From: Jim Meyering <meyering(a)redhat.com>
* fish/fish.c: ... getopt_long already does that. Instead, suggest
"Try `guestfish --help' for more information."
---
fish/fish.c | 2 --
1 files changed, 0 insertions(+), 2 deletions(-)
diff --git a/fish/fish.c b/fish/fish.c
index 8ea1c49..964c906 100644
--- a/fish/fish.c
+++ b/fish/fish.c
@@ -320,8 +320,6 @@ main (int argc, char *argv[])
usage (0);
default:
- fprintf (stderr, _("%s: unexpected command line option 0x%x\n"),
- program_name, c);
usage (1);
}
}
--
1.6.4.378.g88f2f
5:21 a.m.
New subject: [PATCH libguestfs 4/5] guestfish: diagnose stdout write failure
From: Jim Meyering <meyering(a)redhat.com>
Use gnulib's closeout module to ensure any failure to write to
stdout is detected and reported.
* fish/fish.c: Include "closeout.h".
(main): Call atexit (close_stdout);
* bootstrap (modules): Add closeout.
(emit print_*_indent): Likewise, s/int/unsigned int/
---
bootstrap | 1 +
fish/fish.c | 3 +++
2 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/bootstrap b/bootstrap
index 3fd8811..f5f8aaf 100755
--- a/bootstrap
+++ b/bootstrap
@@ -50,6 +50,7 @@ gnulib_tool=$GNULIB_SRCDIR/gnulib-tool
(cd daemon && mkdir -p tests lib && ../$gnulib_tool --update)
modules='
+closeout
gitlog-to-changelog
gnu-make
gnumakefile
diff --git a/fish/fish.c b/fish/fish.c
index 964c906..1790c1a 100644
--- a/fish/fish.c
+++ b/fish/fish.c
@@ -40,6 +40,7 @@
#include <guestfs.h>
#include "fish.h"
+#include "closeout.h"
#include "progname.h"
struct mp {
@@ -143,6 +144,8 @@ main (int argc, char *argv[])
/* getopt_long uses argv[0], so give it the sanitized name, too. */
argv[0] = bad_cast (program_name);
+ atexit (close_stdout);
+
enum { HELP_OPTION = CHAR_MAX + 1 };
static const char *options = "a:Df:h::im:nrv?Vx";
--
1.6.4.378.g88f2f
5:21 a.m.
New subject: [PATCH libguestfs 5/5] build: don't define _GNU_SOURCE manually
From: Jim Meyering <meyering(a)redhat.com>
Now that we're using gnulib in earnest, any manual definition
would provoke a redefinition warning.
* fish/fish.c (_GNU_SOURCE): Don't define.
* fish/destpaths.c (_GNU_SOURCE): Likewise.
* src/guestfs.c (_GNU_SOURCE): Likewise.
* bootstrap (modules): Add asprintf, strchrnul, strerror, strndup
and vasprintf.
* fish/fish.c (main): Set argv[0] to sanitized program_name, so
functions like getopt_long that use argv[0] use the clean name.
---
bootstrap | 7 ++++++-
fish/destpaths.c | 2 --
fish/fish.c | 2 --
src/guestfs.c | 1 -
4 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/bootstrap b/bootstrap
index f5f8aaf..2eba6a7 100755
--- a/bootstrap
+++ b/bootstrap
@@ -50,6 +50,7 @@ gnulib_tool=$GNULIB_SRCDIR/gnulib-tool
(cd daemon && mkdir -p tests lib && ../$gnulib_tool --update)
modules='
+asprintf
closeout
gitlog-to-changelog
gnu-make
@@ -58,8 +59,12 @@ ignore-value
maintainer-makefile
manywarnings
progname
-warnings
+strchrnul
+strerror
+strndup
+vasprintf
vc-list-files
+warnings
'
$gnulib_tool \
diff --git a/fish/destpaths.c b/fish/destpaths.c
index 275db49..1e42ae8 100644
--- a/fish/destpaths.c
+++ b/fish/destpaths.c
@@ -18,8 +18,6 @@
#include <config.h>
-#define _GNU_SOURCE // for strndup, asprintf
-
#include <stdio.h>
#include <stdlib.h>
#include <stddef.h>
diff --git a/fish/fish.c b/fish/fish.c
index 1790c1a..9f0f8df 100644
--- a/fish/fish.c
+++ b/fish/fish.c
@@ -18,8 +18,6 @@
#include <config.h>
-#define _GNU_SOURCE // for strchrnul
-
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
diff --git a/src/guestfs.c b/src/guestfs.c
index 04bd4e8..4a9f11a 100644
--- a/src/guestfs.c
+++ b/src/guestfs.c
@@ -19,7 +19,6 @@
#include <config.h>
#define _BSD_SOURCE /* for mkdtemp, usleep */
-#define _GNU_SOURCE /* for vasprintf, GNU strerror_r, strchrnul */
#include <stdio.h>
#include <stdlib.h>
--
1.6.4.378.g88f2f
5:48 a.m.
New subject: [PATCH libguestfs 1/5] build: avoid some autoconf warnings
On Mon, Aug 24, 2009 at 12:21:53PM +0200, Jim Meyering wrote:
From: Jim Meyering <meyering(a)redhat.com>
* configure.ac: Move gl_EARLY and gl_INIT to be earlier.
---
configure.ac | 10 +++++-----
1 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/configure.ac b/configure.ac
index 5a3379b..2284dac 100644
--- a/configure.ac
+++ b/configure.ac
@@ -31,22 +31,22 @@ AM_SILENT_RULES([yes]) # make --enable-silent-rules the default.
AC_CONFIG_MACRO_DIR([m4])
-AC_PROG_LIBTOOL
-
dnl Split up the version string.
AC_DEFINE([PACKAGE_VERSION_MAJOR],[libguestfs_major],[Major version number])
AC_DEFINE([PACKAGE_VERSION_MINOR],[libguestfs_minor],[Minor version number])
AC_DEFINE([PACKAGE_VERSION_RELEASE],[libguestfs_release],[Release number])
AC_DEFINE([PACKAGE_VERSION_EXTRA],["libguestfs_extra"],[Extra version
string])
+gl_EARLY
+gl_INIT
+
+AC_PROG_LIBTOOL
+
dnl Check for basic C environment.
AC_PROG_CC_STDC
AC_PROG_INSTALL
AC_PROG_CPP
-gl_EARLY
-gl_INIT
-
AC_ARG_ENABLE([gcc-warnings],
[AS_HELP_STRING([--enable-gcc-warnings],
[turn on lots of GCC warnings (for developers)])],
--
1.6.4.378.g88f2f
_______________________________________________
Libguestfs mailing list
Libguestfs(a)redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs
ACK.
Rich.
--
Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones
New in Fedora 11: Fedora Windows cross-compiler. Compile Windows
programs, test, and build Windows installers. Over 70 libraries supprt'd
http://fedoraproject.org/wiki/MinGW http://www.annexia.org/fedora_mingw
5:53 a.m.
New subject: [PATCH libguestfs 2/5] guestfish: write --help to stdout, use gnulib's progname module
On Mon, Aug 24, 2009 at 12:21:54PM +0200, Jim Meyering wrote:
+ /* getopt_long uses argv[0], so give it the sanitized name, too.
*/
+ argv[0] = bad_cast (program_name);
This is wrong.
We rely on seeing the original argv[0] later in the code, so that
people can run ./fish/guestfish from the source directory and have the
appliance path set correctly:
/* If developing, add ./appliance to the path. Note that libtools
* interferes with this because uninstalled guestfish is a shell
* script that runs the real program with an absolute path. Detect
* that too.
*
* BUT if LIBGUESTFS_PATH environment variable is already set by
* the user, then don't override it.
*/
if (getenv ("LIBGUESTFS_PATH") == NULL &&
argv[0] &&
(argv[0][0] != '/' || strstr (argv[0], "/.libs/lt-") != NULL))
guestfs_set_path (g, "appliance:" GUESTFS_DEFAULT_PATH);
+ enum { HELP_OPTION = CHAR_MAX + 1 };
+
static const char *options = "a:Df:h::im:nrv?Vx";
static const struct option long_options[] = {
{ "add", 1, 0, 'a' },
{ "cmd-help", 2, 0, 'h' },
{ "file", 1, 0, 'f' },
- { "help", 0, 0, '?' },
+ { "help", 0, 0, HELP_OPTION },
I'm unclear on why we do this - what's wrong with using '?'?
Rich.
--
Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones
virt-top is 'top' for virtual machines. Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://et.redhat.com/~rjones/virt-top
5:53 a.m.
New subject: [PATCH libguestfs 3/5] guestfish: don't try to diagnose getopt failure
On Mon, Aug 24, 2009 at 12:21:55PM +0200, Jim Meyering wrote:
From: Jim Meyering <meyering(a)redhat.com>
* fish/fish.c: ... getopt_long already does that. Instead, suggest
"Try `guestfish --help' for more information."
---
fish/fish.c | 2 --
1 files changed, 0 insertions(+), 2 deletions(-)
diff --git a/fish/fish.c b/fish/fish.c
index 8ea1c49..964c906 100644
--- a/fish/fish.c
+++ b/fish/fish.c
@@ -320,8 +320,6 @@ main (int argc, char *argv[])
usage (0);
default:
- fprintf (stderr, _("%s: unexpected command line option 0x%x\n"),
- program_name, c);
usage (1);
}
}
--
1.6.4.378.g88f2f
If the previous patch is fixed, then this looks good - ACK.
Rich.
--
Richard Jones, Emerging Technologies, Red Hat http://et.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
5:54 a.m.
New subject: [PATCH libguestfs 4/5] guestfish: diagnose stdout write failure
On Mon, Aug 24, 2009 at 12:21:56PM +0200, Jim Meyering wrote:
From: Jim Meyering <meyering(a)redhat.com>
Use gnulib's closeout module to ensure any failure to write to
stdout is detected and reported.
* fish/fish.c: Include "closeout.h".
(main): Call atexit (close_stdout);
* bootstrap (modules): Add closeout.
(emit print_*_indent): Likewise, s/int/unsigned int/
---
bootstrap | 1 +
fish/fish.c | 3 +++
2 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/bootstrap b/bootstrap
index 3fd8811..f5f8aaf 100755
--- a/bootstrap
+++ b/bootstrap
@@ -50,6 +50,7 @@ gnulib_tool=$GNULIB_SRCDIR/gnulib-tool
(cd daemon && mkdir -p tests lib && ../$gnulib_tool --update)
modules='
+closeout
gitlog-to-changelog
gnu-make
gnumakefile
diff --git a/fish/fish.c b/fish/fish.c
index 964c906..1790c1a 100644
--- a/fish/fish.c
+++ b/fish/fish.c
@@ -40,6 +40,7 @@
#include <guestfs.h>
#include "fish.h"
+#include "closeout.h"
#include "progname.h"
struct mp {
@@ -143,6 +144,8 @@ main (int argc, char *argv[])
/* getopt_long uses argv[0], so give it the sanitized name, too. */
argv[0] = bad_cast (program_name);
+ atexit (close_stdout);
+
enum { HELP_OPTION = CHAR_MAX + 1 };
static const char *options = "a:Df:h::im:nrv?Vx";
--
1.6.4.378.g88f2f
ACK.
Rich.
--
Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones
Read my programming blog: http://rwmj.wordpress.com
Fedora now supports 75 OCaml packages (the OPEN alternative to F#)
http://cocan.org/getting_started_with_ocaml_on_red_hat_and_fedora
5:55 a.m.
New subject: [PATCH libguestfs 5/5] build: don't define _GNU_SOURCE manually
On Mon, Aug 24, 2009 at 12:21:57PM +0200, Jim Meyering wrote:
From: Jim Meyering <meyering(a)redhat.com>
Now that we're using gnulib in earnest, any manual definition
would provoke a redefinition warning.
* fish/fish.c (_GNU_SOURCE): Don't define.
* fish/destpaths.c (_GNU_SOURCE): Likewise.
* src/guestfs.c (_GNU_SOURCE): Likewise.
* bootstrap (modules): Add asprintf, strchrnul, strerror, strndup
and vasprintf.
* fish/fish.c (main): Set argv[0] to sanitized program_name, so
functions like getopt_long that use argv[0] use the clean name.
---
bootstrap | 7 ++++++-
fish/destpaths.c | 2 --
fish/fish.c | 2 --
src/guestfs.c | 1 -
4 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/bootstrap b/bootstrap
index f5f8aaf..2eba6a7 100755
--- a/bootstrap
+++ b/bootstrap
@@ -50,6 +50,7 @@ gnulib_tool=$GNULIB_SRCDIR/gnulib-tool
(cd daemon && mkdir -p tests lib && ../$gnulib_tool --update)
modules='
+asprintf
closeout
gitlog-to-changelog
gnu-make
@@ -58,8 +59,12 @@ ignore-value
maintainer-makefile
manywarnings
progname
-warnings
+strchrnul
+strerror
+strndup
+vasprintf
vc-list-files
+warnings
'
$gnulib_tool \
diff --git a/fish/destpaths.c b/fish/destpaths.c
index 275db49..1e42ae8 100644
--- a/fish/destpaths.c
+++ b/fish/destpaths.c
@@ -18,8 +18,6 @@
#include <config.h>
-#define _GNU_SOURCE // for strndup, asprintf
-
#include <stdio.h>
#include <stdlib.h>
#include <stddef.h>
diff --git a/fish/fish.c b/fish/fish.c
index 1790c1a..9f0f8df 100644
--- a/fish/fish.c
+++ b/fish/fish.c
@@ -18,8 +18,6 @@
#include <config.h>
-#define _GNU_SOURCE // for strchrnul
-
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
diff --git a/src/guestfs.c b/src/guestfs.c
index 04bd4e8..4a9f11a 100644
--- a/src/guestfs.c
+++ b/src/guestfs.c
@@ -19,7 +19,6 @@
#include <config.h>
#define _BSD_SOURCE /* for mkdtemp, usleep */
-#define _GNU_SOURCE /* for vasprintf, GNU strerror_r, strchrnul */
#include <stdio.h>
#include <stdlib.h>
--
1.6.4.378.g88f2f
_______________________________________________
Libguestfs mailing list
Libguestfs(a)redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs
Yup, ACK.
Rich.
--
Richard Jones, Emerging Technologies, Red Hat http://et.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
6:21 a.m.
New subject: [PATCH libguestfs 2/5] guestfish: write --help to stdout, use gnulib's progname module
Richard W.M. Jones wrote:
On Mon, Aug 24, 2009 at 12:21:54PM +0200, Jim Meyering wrote:
> + /* getopt_long uses argv[0], so give it the sanitized name, too. */
> + argv[0] = bad_cast (program_name);
This is wrong.
We rely on seeing the original argv[0] later in the code, so that
people can run ./fish/guestfish from the source directory and have the
appliance path set correctly:
/* If developing, add ./appliance to the path. Note that libtools
* interferes with this because uninstalled guestfish is a shell
* script that runs the real program with an absolute path. Detect
* that too.
*
* BUT if LIBGUESTFS_PATH environment variable is already set by
* the user, then don't override it.
*/
if (getenv ("LIBGUESTFS_PATH") == NULL &&
argv[0] &&
(argv[0][0] != '/' || strstr (argv[0], "/.libs/lt-") != NULL))
guestfs_set_path (g, "appliance:" GUESTFS_DEFAULT_PATH);
Good catch. I'll adapt by saving argv[0] just before calling
getopt_long, setting it as above, and then restoring the original,
afterwards.
However, bear in mind that determining the absolute name of
the running program is quite a portability rats nest, in general.
The above won't always work.
Also, it is good policy, IMHO, to make it so a program works the same
way no matter where the actual binary happens to be located in a file
system hierarchy.
> + enum { HELP_OPTION = CHAR_MAX + 1 };
> +
> static const char *options = "a:Df:h::im:nrv?Vx";
> static const struct option long_options[] = {
> { "add", 1, 0, 'a' },
> { "cmd-help", 2, 0, 'h' },
> { "file", 1, 0, 'f' },
> - { "help", 0, 0, '?' },
> + { "help", 0, 0, HELP_OPTION },
I'm unclear on why we do this - what's wrong with using '?'?
So that we can distinguish getop-diagnosed failure
(where it returns '?') from --help.
With my patch, only --help prints that output, now.
getopt-diagnosed failures get an abbreviated diagnostic like this:
$ ./guestfish --mount
guestfish: option '--mount' requires an argument
Try `guestfish --help' for more information.
[Exit 1]
6:30 a.m.
New subject: [PATCH libguestfs 2/5] guestfish: write --help to stdout, use gnulib's progname module
On Mon, Aug 24, 2009 at 01:21:39PM +0200, Jim Meyering wrote:
Also, it is good policy, IMHO, to make it so a program works the
same
way no matter where the actual binary happens to be located in a file
system hierarchy.
Well, if you can think of a better way to do it ... I can't.
>> + enum { HELP_OPTION = CHAR_MAX + 1 };
>> +
>> static const char *options = "a:Df:h::im:nrv?Vx";
>> static const struct option long_options[] = {
>> { "add", 1, 0, 'a' },
>> { "cmd-help", 2, 0, 'h' },
>> { "file", 1, 0, 'f' },
>> - { "help", 0, 0, '?' },
>> + { "help", 0, 0, HELP_OPTION },
>
> I'm unclear on why we do this - what's wrong with using '?'?
So that we can distinguish getop-diagnosed failure
(where it returns '?') from --help.
Right, I see.
Rich.
--
Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones
Read my programming blog: http://rwmj.wordpress.com
Fedora now supports 75 OCaml packages (the OPEN alternative to F#)
http://cocan.org/getting_started_with_ocaml_on_red_hat_and_fedora
6:51 a.m.
New subject: [PATCH libguestfs 2/5] guestfish: write --help to stdout, use gnulib's progname module
Jim Meyering wrote:
Richard W.M. Jones wrote:
> On Mon, Aug 24, 2009 at 12:21:54PM +0200, Jim Meyering wrote:
>> + /* getopt_long uses argv[0], so give it the sanitized name, too. */
>> + argv[0] = bad_cast (program_name);
>
> This is wrong.
>
> We rely on seeing the original argv[0] later in the code, so that
> people can run ./fish/guestfish from the source directory and have the
> appliance path set correctly:
>
> /* If developing, add ./appliance to the path. Note that libtools
> * interferes with this because uninstalled guestfish is a shell
> * script that runs the real program with an absolute path. Detect
> * that too.
> *
> * BUT if LIBGUESTFS_PATH environment variable is already set by
> * the user, then don't override it.
> */
> if (getenv ("LIBGUESTFS_PATH") == NULL &&
> argv[0] &&
> (argv[0][0] != '/' || strstr (argv[0], "/.libs/lt-") !=
NULL))
> guestfs_set_path (g, "appliance:" GUESTFS_DEFAULT_PATH);
Good catch. I'll adapt by saving argv[0] just before calling
getopt_long, setting it as above, and then restoring the original,
afterwards.
Here's an incremental patch I'll fold onto the one discussed above:
diff --git a/fish/fish.c b/fish/fish.c
index 9f0f8df..547342f 100644
--- a/fish/fish.c
+++ b/fish/fish.c
@@ -139,9 +139,6 @@ main (int argc, char *argv[])
/* Set global program name that is not polluted with libtool artifacts. */
set_program_name (argv[0]);
- /* getopt_long uses argv[0], so give it the sanitized name, too. */
- argv[0] = bad_cast (program_name);
-
atexit (close_stdout);
enum { HELP_OPTION = CHAR_MAX + 1 };
@@ -205,6 +202,11 @@ main (int argc, char *argv[])
(argv[0][0] != '/' || strstr (argv[0], "/.libs/lt-") != NULL))
guestfs_set_path (g, "appliance:" GUESTFS_DEFAULT_PATH);
+ /* getopt_long uses argv[0], so give it the sanitized name, too.
+ But only temporarily. */
+ char *argv0 = argv[0];
+ argv[0] = bad_cast (program_name);
+
for (;;) {
c = getopt_long (argc, argv, options, long_options, &option_index);
if (c == -1) break;
@@ -325,6 +327,9 @@ main (int argc, char *argv[])
}
}
+ /* Restore original value. */
+ argv[0] = argv0;
+
/* Inspector mode invalidates most of the other arguments. */
if (inspector) {
char cmd[1024];
6:58 a.m.
New subject: [PATCH libguestfs 2/5] guestfish: write --help to stdout, use gnulib's progname module
Jim Meyering wrote:
Richard W.M. Jones wrote:
> On Mon, Aug 24, 2009 at 12:21:54PM +0200, Jim Meyering wrote:
>> + /* getopt_long uses argv[0], so give it the sanitized name, too. */
>> + argv[0] = bad_cast (program_name);
>
> This is wrong.
>
> We rely on seeing the original argv[0] later in the code, so that
> people can run ./fish/guestfish from the source directory and have the
> appliance path set correctly:
>
> /* If developing, add ./appliance to the path. Note that libtools
> * interferes with this because uninstalled guestfish is a shell
> * script that runs the real program with an absolute path. Detect
> * that too.
> *
> * BUT if LIBGUESTFS_PATH environment variable is already set by
> * the user, then don't override it.
> */
> if (getenv ("LIBGUESTFS_PATH") == NULL &&
> argv[0] &&
> (argv[0][0] != '/' || strstr (argv[0], "/.libs/lt-") !=
NULL))
> guestfs_set_path (g, "appliance:" GUESTFS_DEFAULT_PATH);
Good catch. I'll adapt by saving argv[0] just before calling
getopt_long, setting it as above, and then restoring the original,
afterwards.
BTW, here's the complete, adjusted change-set:
From bdc25e068deaaa3f80d5d0dc773b41fa9c84d503 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering(a)redhat.com>
Date: Fri, 21 Aug 2009 19:05:20 +0200
Subject: [PATCH libguestfs 2/5] guestfish: write --help to stdout, use gnulib's
progname module
* fish/fish.c: Include "progname.h".
(main): Call set_program_name to initialize.
Don't hard-code guestfish everywhere. Use program_name.
(usage): Don't spew all of --help for a mis-typed option.
Split long lines.
---
bootstrap | 1 +
fish/Makefile.am | 3 +-
fish/fish.c | 129 ++++++++++++++++++++++++++++++++++++-----------------
3 files changed, 90 insertions(+), 43 deletions(-)
diff --git a/bootstrap b/bootstrap
index 76e2216..3fd8811 100755
--- a/bootstrap
+++ b/bootstrap
@@ -56,6 +56,7 @@ gnumakefile
ignore-value
maintainer-makefile
manywarnings
+progname
warnings
vc-list-files
'
diff --git a/fish/Makefile.am b/fish/Makefile.am
index 94bf757..c8ba3ea 100644
--- a/fish/Makefile.am
+++ b/fish/Makefile.am
@@ -52,13 +52,14 @@ guestfish_CFLAGS = \
-I$(top_srcdir)/src -I$(top_builddir)/src \
-I$(top_srcdir)/fish -I$(top_builddir)/fish \
-DGUESTFS_DEFAULT_PATH='"$(libdir)/guestfs"' \
+ -I$(srcdir)/../gnulib/lib -I../gnulib/lib \
$(WARN_CFLAGS) $(WERROR_CFLAGS)
guestfish_LDADD = $(top_builddir)/src/libguestfs.la $(LIBREADLINE)
# Make libguestfs use the convenience library.
noinst_LTLIBRARIES = librc_protocol.la
-guestfish_LDADD += librc_protocol.la
+guestfish_LDADD += librc_protocol.la ../gnulib/lib/libgnu.la
if HAVE_RPCGEN
rc_protocol.c: rc_protocol.x
diff --git a/fish/fish.c b/fish/fish.c
index 3acd450..baa1b6e 100644
--- a/fish/fish.c
+++ b/fish/fish.c
@@ -40,6 +40,7 @@
#include <guestfs.h>
#include "fish.h"
+#include "progname.h"
struct mp {
struct mp *next;
@@ -87,21 +88,25 @@ launch (guestfs_h *_g)
return 0;
}
-static void
-usage (void)
+static void __attribute__((noreturn))
+usage (int status)
{
- fprintf (stderr,
- _("guestfish: guest filesystem shell\n"
- "guestfish lets you edit virtual machine filesystems\n"
+ if (status != EXIT_SUCCESS)
+ fprintf (stderr, _("Try `%s --help' for more information.\n"),
+ program_name);
+ else {
+ fprintf (stdout,
+ _("%s: guest filesystem shell\n"
+ "%s lets you edit virtual machine filesystems\n"
"Copyright (C) 2009 Red Hat Inc.\n"
"Usage:\n"
- " guestfish [--options] cmd [: cmd : cmd ...]\n"
- " guestfish -i libvirt-domain\n"
- " guestfish -i disk-image(s)\n"
+ " %s [--options] cmd [: cmd : cmd ...]\n"
+ " %s -i libvirt-domain\n"
+ " %s -i disk-image(s)\n"
"or for interactive use:\n"
- " guestfish\n"
+ " %s\n"
"or from a shell script:\n"
- " guestfish <<EOF\n"
+ " %s <<EOF\n"
" cmd\n"
" ...\n"
" EOF\n"
@@ -115,24 +120,34 @@ usage (void)
" --listen Listen for remote commands\n"
" -m|--mount dev[:mnt] Mount dev on mnt (if omitted, /)\n"
" -n|--no-sync Don't autosync\n"
- " --remote[=pid] Send commands to remote guestfish\n"
+ " --remote[=pid] Send commands to remote %s\n"
" -r|--ro Mount read-only\n"
" --selinux Enable SELinux support\n"
" -v|--verbose Verbose messages\n"
" -x Echo each command before executing it\n"
" -V|--version Display version and exit\n"
- "For more information, see the manpage guestfish(1).\n"));
+ "For more information, see the manpage %s(1).\n"),
+ program_name, program_name, program_name,
+ program_name, program_name, program_name,
+ program_name, program_name, program_name);
+ }
+ exit (status);
}
int
main (int argc, char *argv[])
{
+ /* Set global program name that is not polluted with libtool artifacts. */
+ set_program_name (argv[0]);
+
+ enum { HELP_OPTION = CHAR_MAX + 1 };
+
static const char *options = "a:Df:h::im:nrv?Vx";
static const struct option long_options[] = {
{ "add", 1, 0, 'a' },
{ "cmd-help", 2, 0, 'h' },
{ "file", 1, 0, 'f' },
- { "help", 0, 0, '?' },
+ { "help", 0, 0, HELP_OPTION },
{ "inspector", 0, 0, 'i' },
{ "listen", 0, 0, 0 },
{ "mount", 1, 0, 'm' },
@@ -186,6 +201,11 @@ main (int argc, char *argv[])
(argv[0][0] != '/' || strstr (argv[0], "/.libs/lt-") != NULL))
guestfs_set_path (g, "appliance:" GUESTFS_DEFAULT_PATH);
+ /* getopt_long uses argv[0], so give it the sanitized name, too.
+ But only temporarily. */
+ char *argv0 = argv[0];
+ argv[0] = bad_cast (program_name);
+
for (;;) {
c = getopt_long (argc, argv, options, long_options, &option_index);
if (c == -1) break;
@@ -197,21 +217,24 @@ main (int argc, char *argv[])
else if (strcmp (long_options[option_index].name, "remote") == 0) {
if (optarg) {
if (sscanf (optarg, "%d", &remote_control) != 1) {
- fprintf (stderr, _("guestfish: --listen=PID: PID was not a number:
%s\n"), optarg);
+ fprintf (stderr, _("%s: --listen=PID: PID was not a number:
%s\n"),
+ program_name, optarg);
exit (1);
}
} else {
p = getenv ("GUESTFISH_PID");
if (!p || sscanf (p, "%d", &remote_control) != 1) {
- fprintf (stderr, _("guestfish: remote: $GUESTFISH_PID must be set to the
PID of the remote process\n"));
+ fprintf (stderr, _("%s: remote: $GUESTFISH_PID must be set"
+ " to the PID of the remote process\n"),
+ program_name);
exit (1);
}
}
} else if (strcmp (long_options[option_index].name, "selinux") == 0) {
guestfs_set_selinux (g, 1);
} else {
- fprintf (stderr, _("guestfish: unknown long option: %s (%d)\n"),
- long_options[option_index].name, option_index);
+ fprintf (stderr, _("%s: unknown long option: %s (%d)\n"),
+ program_name, long_options[option_index].name, option_index);
exit (1);
}
break;
@@ -237,7 +260,8 @@ main (int argc, char *argv[])
case 'f':
if (file) {
- fprintf (stderr, _("guestfish: only one -f parameter can be
given\n"));
+ fprintf (stderr, _("%s: only one -f parameter can be given\n"),
+ program_name);
exit (1);
}
file = optarg;
@@ -287,24 +311,26 @@ main (int argc, char *argv[])
break;
case 'V':
- printf ("guestfish %s\n", PACKAGE_VERSION);
+ printf ("%s %s\n", program_name, PACKAGE_VERSION);
exit (0);
case 'x':
echo_commands = 1;
break;
- case '?':
- usage ();
- exit (0);
+ case HELP_OPTION:
+ usage (0);
default:
- fprintf (stderr, _("guestfish: unexpected command line option 0x%x\n"),
- c);
- exit (1);
+ fprintf (stderr, _("%s: unexpected command line option 0x%x\n"),
+ program_name, c);
+ usage (1);
}
}
+ /* Restore original value. */
+ argv[0] = argv0;
+
/* Inspector mode invalidates most of the other arguments. */
if (inspector) {
char cmd[1024];
@@ -312,11 +338,15 @@ main (int argc, char *argv[])
if (drvs || mps || remote_control_listen || remote_control ||
guestfs_get_selinux (g)) {
- fprintf (stderr, _("guestfish: cannot use -i option with -a, -m, --listen,
--remote or --selinux\n"));
+ fprintf (stderr, _("%s: cannot use -i option with -a, -m,"
+ " --listen, --remote or --selinux\n"),
+ program_name);
exit (1);
}
if (optind >= argc) {
- fprintf (stderr, _("guestfish -i requires a libvirt domain or path(s) to disk
image(s)\n"));
+ fprintf (stderr,
+ _("%s: -i requires a libvirt domain or path(s) to disk
image(s)\n"),
+ program_name);
exit (1);
}
@@ -324,7 +354,9 @@ main (int argc, char *argv[])
while (optind < argc) {
if (strlen (cmd) + strlen (argv[optind]) + strlen (argv[0]) + 60
= sizeof cmd) {
- fprintf (stderr, _("guestfish:
virt-inspector command too long for fixed-size buffer\n"));
+ fprintf (stderr,
+ _("%s: virt-inspector command too long for fixed-size
buffer\n"),
+ program_name);
exit (1);
}
strcat (cmd, " '");
@@ -347,7 +379,7 @@ main (int argc, char *argv[])
if (verbose)
fprintf (stderr,
- "guestfish -i: running virt-inspector command:\n%s\n", cmd);
+ "%s -i: running virt-inspector command:\n%s\n", program_name,
cmd);
r = system (cmd);
if (r == -1) {
@@ -368,17 +400,23 @@ main (int argc, char *argv[])
/* Remote control? */
if (remote_control_listen && remote_control) {
- fprintf (stderr, _("guestfish: cannot use --listen and --remote options at the
same time\n"));
+ fprintf (stderr,
+ _("%s: cannot use --listen and --remote options at the same
time\n"),
+ program_name);
exit (1);
}
if (remote_control_listen) {
if (optind < argc) {
- fprintf (stderr, _("guestfish: extra parameters on the command line with
--listen flag\n"));
+ fprintf (stderr,
+ _("%s: extra parameters on the command line with --listen
flag\n"),
+ program_name);
exit (1);
}
if (file) {
- fprintf (stderr, _("guestfish: cannot use --listen and --file options at the
same time\n"));
+ fprintf (stderr,
+ _("%s: cannot use --listen and --file options at the same
time\n"),
+ program_name);
exit (1);
}
rc_listen ();
@@ -602,12 +640,14 @@ script (int prompt)
p++;
len = strcspn (p, "\"");
if (p[len] == '\0') {
- fprintf (stderr, _("guestfish: unterminated double quote\n"));
+ fprintf (stderr, _("%s: unterminated double quote\n"),
program_name);
if (exit_on_error) exit (1);
goto next_command;
}
if (p[len+1] && (p[len+1] != ' ' && p[len+1] !=
'\t')) {
- fprintf (stderr, _("guestfish: command arguments not separated by
whitespace\n"));
+ fprintf (stderr,
+ _("%s: command arguments not separated by whitespace\n"),
+ program_name);
if (exit_on_error) exit (1);
goto next_command;
}
@@ -617,12 +657,14 @@ script (int prompt)
p++;
len = strcspn (p, "'");
if (p[len] == '\0') {
- fprintf (stderr, _("guestfish: unterminated single quote\n"));
+ fprintf (stderr, _("%s: unterminated single quote\n"),
program_name);
if (exit_on_error) exit (1);
goto next_command;
}
if (p[len+1] && (p[len+1] != ' ' && p[len+1] !=
'\t')) {
- fprintf (stderr, _("guestfish: command arguments not separated by
whitespace\n"));
+ fprintf (stderr,
+ _("%s: command arguments not separated by whitespace\n"),
+ program_name);
if (exit_on_error) exit (1);
goto next_command;
}
@@ -643,12 +685,15 @@ script (int prompt)
pend++;
}
if (c != 0) {
- fprintf (stderr, _("guestfish: unterminated \"[...]\"
sequence\n"));
+ fprintf (stderr,
+ _("%s: unterminated \"[...]\" sequence\n"),
program_name);
if (exit_on_error) exit (1);
goto next_command;
}
if (*pend && (*pend != ' ' && *pend != '\t')) {
- fprintf (stderr, _("guestfish: command arguments not separated by
whitespace\n"));
+ fprintf (stderr,
+ _("%s: command arguments not separated by whitespace\n"),
+ program_name);
if (exit_on_error) exit (1);
goto next_command;
}
@@ -667,8 +712,8 @@ script (int prompt)
} else
pend = &p[len];
} else {
- fprintf (stderr, _("guestfish: internal error parsing string at
'%s'\n"),
- p);
+ fprintf (stderr, _("%s: internal error parsing string at
'%s'\n"),
+ program_name, p);
abort ();
}
@@ -684,7 +729,7 @@ script (int prompt)
}
if (i == sizeof argv / sizeof argv[0]) {
- fprintf (stderr, _("guestfish: too many arguments\n"));
+ fprintf (stderr, _("%s: too many arguments\n"), program_name);
if (exit_on_error) exit (1);
goto next_command;
}
@@ -713,7 +758,7 @@ cmdline (char *argv[], int optind, int argc)
cmd = argv[optind++];
if (strcmp (cmd, ":") == 0) {
- fprintf (stderr, _("guestfish: empty command on command line\n"));
+ fprintf (stderr, _("%s: empty command on command line\n"), program_name);
exit (1);
}
params = &argv[optind];
--
1.6.4.378.g88f2f
7:11 a.m.
New subject: [PATCH libguestfs 2/5] guestfish: write --help to stdout, use gnulib's progname module
On Mon, Aug 24, 2009 at 01:51:18PM +0200, Jim Meyering wrote:
@@ -205,6 +202,11 @@ main (int argc, char *argv[])
(argv[0][0] != '/' || strstr (argv[0], "/.libs/lt-") != NULL))
guestfs_set_path (g, "appliance:" GUESTFS_DEFAULT_PATH);
+ /* getopt_long uses argv[0], so give it the sanitized name, too.
+ But only temporarily. */
+ char *argv0 = argv[0];
+ argv[0] = bad_cast (program_name);
+
for (;;) {
c = getopt_long (argc, argv, options, long_options, &option_index);
if (c == -1) break;
@@ -325,6 +327,9 @@ main (int argc, char *argv[])
}
}
+ /* Restore original value. */
+ argv[0] = argv0;
+
/* Inspector mode invalidates most of the other arguments. */
if (inspector) {
char cmd[1024];
I'm maybe missing something, but why restore it?
How about instead something like:
char *real_argv0 = argv[0];
argv[0] = bad_cast (program_name);
//...
/* change the path-munging code to use real_argv0 */
//...
/* don't bother restoring argv[0] */
?
Rich.
--
Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine. Supports Linux and Windows.
http://et.redhat.com/~rjones/virt-df/
7:11 a.m.
New subject: [PATCH libguestfs 2/5] guestfish: write --help to stdout, use gnulib's progname module
On Mon, Aug 24, 2009 at 12:30:30PM +0100, Richard W.M. Jones wrote:
On Mon, Aug 24, 2009 at 01:21:39PM +0200, Jim Meyering wrote:
> Also, it is good policy, IMHO, to make it so a program works the same
> way no matter where the actual binary happens to be located in a file
> system hierarchy.
Well, if you can think of a better way to do it ... I can't.
> >> + enum { HELP_OPTION = CHAR_MAX + 1 };
> >> +
> >> static const char *options = "a:Df:h::im:nrv?Vx";
> >> static const struct option long_options[] = {
> >> { "add", 1, 0, 'a' },
> >> { "cmd-help", 2, 0, 'h' },
> >> { "file", 1, 0, 'f' },
> >> - { "help", 0, 0, '?' },
> >> + { "help", 0, 0, HELP_OPTION },
> >
> > I'm unclear on why we do this - what's wrong with using '?'?
>
> So that we can distinguish getop-diagnosed failure
> (where it returns '?') from --help.
Right, I see.
That was ACK BTW.
Rich.
--
Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine. Supports Linux and Windows.
http://et.redhat.com/~rjones/virt-df/
7:12 a.m.
New subject: [PATCH libguestfs 2/5] guestfish: write --help to stdout, use gnulib's progname module
On Mon, Aug 24, 2009 at 01:11:51PM +0100, Richard W.M. Jones wrote:
On Mon, Aug 24, 2009 at 12:30:30PM +0100, Richard W.M. Jones wrote:
> On Mon, Aug 24, 2009 at 01:21:39PM +0200, Jim Meyering wrote:
> > Also, it is good policy, IMHO, to make it so a program works the same
> > way no matter where the actual binary happens to be located in a file
> > system hierarchy.
>
> Well, if you can think of a better way to do it ... I can't.
>
> > >> + enum { HELP_OPTION = CHAR_MAX + 1 };
> > >> +
> > >> static const char *options = "a:Df:h::im:nrv?Vx";
> > >> static const struct option long_options[] = {
> > >> { "add", 1, 0, 'a' },
> > >> { "cmd-help", 2, 0, 'h' },
> > >> { "file", 1, 0, 'f' },
> > >> - { "help", 0, 0, '?' },
> > >> + { "help", 0, 0, HELP_OPTION },
> > >
> > > I'm unclear on why we do this - what's wrong with using
'?'?
> >
> > So that we can distinguish getop-diagnosed failure
> > (where it returns '?') from --help.
>
> Right, I see.
That was ACK BTW.
.. or rather, ACK to this part of the patch.
Rich.
--
Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones
libguestfs lets you edit virtual machines. Supports shell scripting,
bindings from many languages. http://et.redhat.com/~rjones/libguestfs/
See what it can do: http://et.redhat.com/~rjones/libguestfs/recipes.html
7:21 a.m.
New subject: [PATCH libguestfs 2/5] guestfish: write --help to stdout, use gnulib's progname module
Richard W.M. Jones wrote:
On Mon, Aug 24, 2009 at 01:51:18PM +0200, Jim Meyering wrote:
> @@ -205,6 +202,11 @@ main (int argc, char *argv[])
> (argv[0][0] != '/' || strstr (argv[0], "/.libs/lt-") !=
NULL))
> guestfs_set_path (g, "appliance:" GUESTFS_DEFAULT_PATH);
>
> + /* getopt_long uses argv[0], so give it the sanitized name, too.
> + But only temporarily. */
> + char *argv0 = argv[0];
> + argv[0] = bad_cast (program_name);
> +
> for (;;) {
> c = getopt_long (argc, argv, options, long_options, &option_index);
> if (c == -1) break;
> @@ -325,6 +327,9 @@ main (int argc, char *argv[])
> }
> }
>
> + /* Restore original value. */
> + argv[0] = argv0;
> +
> /* Inspector mode invalidates most of the other arguments. */
> if (inspector) {
> char cmd[1024];
I'm maybe missing something, but why restore it?
How about instead something like:
char *real_argv0 = argv[0];
argv[0] = bad_cast (program_name);
//...
/* change the path-munging code to use real_argv0 */
//...
/* don't bother restoring argv[0] */
Sure. That works, too.
Here's the incremental:
diff --git a/fish/fish.c b/fish/fish.c
index baa1b6e..e799f86 100644
--- a/fish/fish.c
+++ b/fish/fish.c
@@ -203,7 +203,7 @@ main (int argc, char *argv[])
/* getopt_long uses argv[0], so give it the sanitized name, too.
But only temporarily. */
- char *argv0 = argv[0];
+ char *real_argv0 = argv[0];
argv[0] = bad_cast (program_name);
for (;;) {
@@ -328,9 +328,6 @@ main (int argc, char *argv[])
}
}
- /* Restore original value. */
- argv[0] = argv0;
-
/* Inspector mode invalidates most of the other arguments. */
if (inspector) {
char cmd[1024];
@@ -352,7 +349,7 @@ main (int argc, char *argv[])
strcpy (cmd, "a=`virt-inspector");
while (optind < argc) {
- if (strlen (cmd) + strlen (argv[optind]) + strlen (argv[0]) + 60
+ if (strlen (cmd) + strlen (argv[optind]) + strlen (real_argv0) + 60
= sizeof cmd) {
fprintf (stderr,
_("%s: virt-inspector command too long for fixed-size
buffer\n"),
@@ -370,7 +367,7 @@ main (int argc, char *argv[])
else
strcat (cmd, " --fish");
- sprintf (&cmd[strlen(cmd)], "` && %s $a", argv[0]);
+ sprintf (&cmd[strlen(cmd)], "` && %s $a", real_argv0);
if (guestfs_get_verbose (g))
strcat (cmd, " -v");
7:59 a.m.
New subject: [PATCH libguestfs 2/5] guestfish: write --help to stdout, use gnulib's progname module
On Mon, Aug 24, 2009 at 02:21:51PM +0200, Jim Meyering wrote:
Sure. That works, too.
Here's the incremental:
No, this doesn't fix the path munging to use real_argv0.
Rich.
--
Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones
virt-top is 'top' for virtual machines. Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://et.redhat.com/~rjones/virt-top
9:26 a.m.
New subject: [PATCH libguestfs 2/5] guestfish: write --help to stdout, use gnulib's progname module
Richard W.M. Jones wrote:
On Mon, Aug 24, 2009 at 02:21:51PM +0200, Jim Meyering wrote:
> Sure. That works, too.
> Here's the incremental:
No, this doesn't fix the path munging to use real_argv0.
"fix"?
Is something incorrect, or is it just that you'd prefer
to move the definition of real_argv0 "up"?
I see only two uses of argv[0] in main:
- path-related stuff *before* I change argv[0] and define real_argv0
if (getenv ("LIBGUESTFS_PATH") == NULL &&
argv[0] &&
(argv[0][0] != '/' || strstr (argv[0], "/.libs/lt-") != NULL))
guestfs_set_path (g, "appliance:" GUESTFS_DEFAULT_PATH);
- then, this code, afterwards, that I did change to use real_argv0:
strcpy (cmd, "a=`virt-inspector");
while (optind < argc) {
if (strlen (cmd) + strlen (argv[optind]) + strlen (real_argv0) + 60
...
sprintf (&cmd[strlen(cmd)], "` && %s $a", real_argv0);
If you'd prefer to move the definition of real_argv0 to precede
the former, let me know, and I'll move it and adjust.
However, note that that has no impact on correctness.
9:38 a.m.
New subject: [PATCH libguestfs 2/5] guestfish: write --help to stdout, use gnulib's progname module
On Mon, Aug 24, 2009 at 04:26:16PM +0200, Jim Meyering wrote:
Richard W.M. Jones wrote:
> On Mon, Aug 24, 2009 at 02:21:51PM +0200, Jim Meyering wrote:
>> Sure. That works, too.
>> Here's the incremental:
>
> No, this doesn't fix the path munging to use real_argv0.
"fix"?
Is something incorrect, or is it just that you'd prefer
to move the definition of real_argv0 "up"?
I see only two uses of argv[0] in main:
- path-related stuff *before* I change argv[0] and define real_argv0
if (getenv ("LIBGUESTFS_PATH") == NULL &&
argv[0] &&
(argv[0][0] != '/' || strstr (argv[0], "/.libs/lt-") != NULL))
guestfs_set_path (g, "appliance:" GUESTFS_DEFAULT_PATH);
- then, this code, afterwards, that I did change to use real_argv0:
strcpy (cmd, "a=`virt-inspector");
while (optind < argc) {
if (strlen (cmd) + strlen (argv[optind]) + strlen (real_argv0) + 60
...
sprintf (&cmd[strlen(cmd)], "` && %s $a", real_argv0);
If you'd prefer to move the definition of real_argv0 to precede
the former, let me know, and I'll move it and adjust.
However, note that that has no impact on correctness.
You're right - for some reason I thought the path-munging code
occurred after overwriting argv[0].
In general though I think the code is now much more prone to errors if
we rearrange the code in future. (It's already complex and prone to
introducing errors).
If the only benefit to rewriting argv[0] is that we get a slightly
better error message from getopt, then I'm inclined not to do it at
all. Leave all the argv[0] code as it is, and change anything else to
use program_name.
Rich.
--
Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones
libguestfs lets you edit virtual machines. Supports shell scripting,
bindings from many languages. http://et.redhat.com/~rjones/libguestfs/
See what it can do: http://et.redhat.com/~rjones/libguestfs/recipes.html
10:46 a.m.
New subject: [PATCH libguestfs 2/5] guestfish: write --help to stdout, use gnulib's progname module
Richard W.M. Jones wrote:
On Mon, Aug 24, 2009 at 04:26:16PM +0200, Jim Meyering wrote:
> Richard W.M. Jones wrote:
>
> > On Mon, Aug 24, 2009 at 02:21:51PM +0200, Jim Meyering wrote:
> >> Sure. That works, too.
> >> Here's the incremental:
> >
> > No, this doesn't fix the path munging to use real_argv0.
>
> "fix"?
> Is something incorrect, or is it just that you'd prefer
> to move the definition of real_argv0 "up"?
>
> I see only two uses of argv[0] in main:
>
> - path-related stuff *before* I change argv[0] and define real_argv0
>
> if (getenv ("LIBGUESTFS_PATH") == NULL &&
> argv[0] &&
> (argv[0][0] != '/' || strstr (argv[0], "/.libs/lt-") !=
NULL))
> guestfs_set_path (g, "appliance:" GUESTFS_DEFAULT_PATH);
>
> - then, this code, afterwards, that I did change to use real_argv0:
>
> strcpy (cmd, "a=`virt-inspector");
> while (optind < argc) {
> if (strlen (cmd) + strlen (argv[optind]) + strlen (real_argv0) + 60
> ...
> sprintf (&cmd[strlen(cmd)], "` && %s $a", real_argv0);
>
> If you'd prefer to move the definition of real_argv0 to precede
> the former, let me know, and I'll move it and adjust.
> However, note that that has no impact on correctness.
You're right - for some reason I thought the path-munging code
occurred after overwriting argv[0].
In general though I think the code is now much more prone to errors if
we rearrange the code in future. (It's already complex and prone to
introducing errors).
If the only benefit to rewriting argv[0] is that we get a slightly
better error message from getopt, then I'm inclined not to do it at
all. Leave all the argv[0] code as it is, and change anything else to
use program_name.
There are a couple of compelling arguments for making the
change in general:
- testing and reproducibility: with the change, all diagnostics
start with the constant string, "program_name: ".
Otherwise, you'll have most that are that way, yet a few
that then start with an absolute directory name and end in
"/.libs/lt-program_name: ". Worse still, in some environments,
that leading directory name will vary from run to run depending
on mount point names.
- overall uniformity. in building the tools and testing them, we've
seen how much output you may have to wade through when diagnosing a
problem. If we ensure that all diagnostics start with "program_name: "
it's that much easier to be sure that a quick use of grep finds all
error messages.
If it will make it easier to accept, I will add some small tests
that verify the expected behavior.
An alternative is not to modify argv at all, but
rather to make a copy of it, change argv_copy[0],
and to pass argv_copy to getopt_long.
Let me know.
10:58 a.m.
New subject: [PATCH libguestfs 2/5] guestfish: write --help to stdout, use gnulib's progname module
On Mon, Aug 24, 2009 at 05:46:03PM +0200, Jim Meyering wrote:
There are a couple of compelling arguments for making the
change in general:
- testing and reproducibility: with the change, all diagnostics
start with the constant string, "program_name: ".
Otherwise, you'll have most that are that way, yet a few
that then start with an absolute directory name and end in
"/.libs/lt-program_name: ". Worse still, in some environments,
that leading directory name will vary from run to run depending
on mount point names.
- overall uniformity. in building the tools and testing them, we've
seen how much output you may have to wade through when diagnosing a
problem. If we ensure that all diagnostics start with "program_name: "
it's that much easier to be sure that a quick use of grep finds all
error messages.
If it will make it easier to accept, I will add some small tests
that verify the expected behavior.
Well OK. What's the final version of this patch (not incremental)?
Rich.
--
Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones
New in Fedora 11: Fedora Windows cross-compiler. Compile Windows
programs, test, and build Windows installers. Over 70 libraries supprt'd
http://fedoraproject.org/wiki/MinGW http://www.annexia.org/fedora_mingw
11 a.m.
New subject: [PATCH libguestfs 2/5] guestfish: write --help to stdout, use gnulib's progname module
Richard W.M. Jones wrote:
On Mon, Aug 24, 2009 at 05:46:03PM +0200, Jim Meyering wrote:
> There are a couple of compelling arguments for making the
> change in general:
>
> - testing and reproducibility: with the change, all diagnostics
> start with the constant string, "program_name: ".
> Otherwise, you'll have most that are that way, yet a few
> that then start with an absolute directory name and end in
> "/.libs/lt-program_name: ". Worse still, in some environments,
> that leading directory name will vary from run to run depending
> on mount point names.
>
> - overall uniformity. in building the tools and testing them, we've
> seen how much output you may have to wade through when diagnosing a
> problem. If we ensure that all diagnostics start with "program_name:
"
> it's that much easier to be sure that a quick use of grep finds all
> error messages.
>
> If it will make it easier to accept, I will add some small tests
> that verify the expected behavior.
Well OK. What's the final version of this patch (not incremental)?
Here it is:
From 5fbc1b89dc7d23171565eb6f28105ac001d5cacd Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering(a)redhat.com>
Date: Fri, 21 Aug 2009 19:05:20 +0200
Subject: [PATCH libguestfs 2/5] guestfish: write --help to stdout, use gnulib's
progname module
* fish/fish.c: Include "progname.h".
(main): Call set_program_name to initialize.
Don't hard-code guestfish everywhere. Use program_name.
However, be careful when modifying argv[0], since it is used
in the hopes that it is an absolute file name.
(usage): Don't spew all of --help for a mis-typed option.
Split long lines.
---
bootstrap | 1 +
fish/Makefile.am | 3 +-
fish/fish.c | 130 +++++++++++++++++++++++++++++++++++------------------
3 files changed, 89 insertions(+), 45 deletions(-)
diff --git a/bootstrap b/bootstrap
index 76e2216..3fd8811 100755
--- a/bootstrap
+++ b/bootstrap
@@ -56,6 +56,7 @@ gnumakefile
ignore-value
maintainer-makefile
manywarnings
+progname
warnings
vc-list-files
'
diff --git a/fish/Makefile.am b/fish/Makefile.am
index 94bf757..c8ba3ea 100644
--- a/fish/Makefile.am
+++ b/fish/Makefile.am
@@ -52,13 +52,14 @@ guestfish_CFLAGS = \
-I$(top_srcdir)/src -I$(top_builddir)/src \
-I$(top_srcdir)/fish -I$(top_builddir)/fish \
-DGUESTFS_DEFAULT_PATH='"$(libdir)/guestfs"' \
+ -I$(srcdir)/../gnulib/lib -I../gnulib/lib \
$(WARN_CFLAGS) $(WERROR_CFLAGS)
guestfish_LDADD = $(top_builddir)/src/libguestfs.la $(LIBREADLINE)
# Make libguestfs use the convenience library.
noinst_LTLIBRARIES = librc_protocol.la
-guestfish_LDADD += librc_protocol.la
+guestfish_LDADD += librc_protocol.la ../gnulib/lib/libgnu.la
if HAVE_RPCGEN
rc_protocol.c: rc_protocol.x
diff --git a/fish/fish.c b/fish/fish.c
index 3acd450..e799f86 100644
--- a/fish/fish.c
+++ b/fish/fish.c
@@ -40,6 +40,7 @@
#include <guestfs.h>
#include "fish.h"
+#include "progname.h"
struct mp {
struct mp *next;
@@ -87,21 +88,25 @@ launch (guestfs_h *_g)
return 0;
}
-static void
-usage (void)
+static void __attribute__((noreturn))
+usage (int status)
{
- fprintf (stderr,
- _("guestfish: guest filesystem shell\n"
- "guestfish lets you edit virtual machine filesystems\n"
+ if (status != EXIT_SUCCESS)
+ fprintf (stderr, _("Try `%s --help' for more information.\n"),
+ program_name);
+ else {
+ fprintf (stdout,
+ _("%s: guest filesystem shell\n"
+ "%s lets you edit virtual machine filesystems\n"
"Copyright (C) 2009 Red Hat Inc.\n"
"Usage:\n"
- " guestfish [--options] cmd [: cmd : cmd ...]\n"
- " guestfish -i libvirt-domain\n"
- " guestfish -i disk-image(s)\n"
+ " %s [--options] cmd [: cmd : cmd ...]\n"
+ " %s -i libvirt-domain\n"
+ " %s -i disk-image(s)\n"
"or for interactive use:\n"
- " guestfish\n"
+ " %s\n"
"or from a shell script:\n"
- " guestfish <<EOF\n"
+ " %s <<EOF\n"
" cmd\n"
" ...\n"
" EOF\n"
@@ -115,24 +120,34 @@ usage (void)
" --listen Listen for remote commands\n"
" -m|--mount dev[:mnt] Mount dev on mnt (if omitted, /)\n"
" -n|--no-sync Don't autosync\n"
- " --remote[=pid] Send commands to remote guestfish\n"
+ " --remote[=pid] Send commands to remote %s\n"
" -r|--ro Mount read-only\n"
" --selinux Enable SELinux support\n"
" -v|--verbose Verbose messages\n"
" -x Echo each command before executing it\n"
" -V|--version Display version and exit\n"
- "For more information, see the manpage guestfish(1).\n"));
+ "For more information, see the manpage %s(1).\n"),
+ program_name, program_name, program_name,
+ program_name, program_name, program_name,
+ program_name, program_name, program_name);
+ }
+ exit (status);
}
int
main (int argc, char *argv[])
{
+ /* Set global program name that is not polluted with libtool artifacts. */
+ set_program_name (argv[0]);
+
+ enum { HELP_OPTION = CHAR_MAX + 1 };
+
static const char *options = "a:Df:h::im:nrv?Vx";
static const struct option long_options[] = {
{ "add", 1, 0, 'a' },
{ "cmd-help", 2, 0, 'h' },
{ "file", 1, 0, 'f' },
- { "help", 0, 0, '?' },
+ { "help", 0, 0, HELP_OPTION },
{ "inspector", 0, 0, 'i' },
{ "listen", 0, 0, 0 },
{ "mount", 1, 0, 'm' },
@@ -186,6 +201,11 @@ main (int argc, char *argv[])
(argv[0][0] != '/' || strstr (argv[0], "/.libs/lt-") != NULL))
guestfs_set_path (g, "appliance:" GUESTFS_DEFAULT_PATH);
+ /* getopt_long uses argv[0], so give it the sanitized name, too.
+ But only temporarily. */
+ char *real_argv0 = argv[0];
+ argv[0] = bad_cast (program_name);
+
for (;;) {
c = getopt_long (argc, argv, options, long_options, &option_index);
if (c == -1) break;
@@ -197,21 +217,24 @@ main (int argc, char *argv[])
else if (strcmp (long_options[option_index].name, "remote") == 0) {
if (optarg) {
if (sscanf (optarg, "%d", &remote_control) != 1) {
- fprintf (stderr, _("guestfish: --listen=PID: PID was not a number:
%s\n"), optarg);
+ fprintf (stderr, _("%s: --listen=PID: PID was not a number:
%s\n"),
+ program_name, optarg);
exit (1);
}
} else {
p = getenv ("GUESTFISH_PID");
if (!p || sscanf (p, "%d", &remote_control) != 1) {
- fprintf (stderr, _("guestfish: remote: $GUESTFISH_PID must be set to the
PID of the remote process\n"));
+ fprintf (stderr, _("%s: remote: $GUESTFISH_PID must be set"
+ " to the PID of the remote process\n"),
+ program_name);
exit (1);
}
}
} else if (strcmp (long_options[option_index].name, "selinux") == 0) {
guestfs_set_selinux (g, 1);
} else {
- fprintf (stderr, _("guestfish: unknown long option: %s (%d)\n"),
- long_options[option_index].name, option_index);
+ fprintf (stderr, _("%s: unknown long option: %s (%d)\n"),
+ program_name, long_options[option_index].name, option_index);
exit (1);
}
break;
@@ -237,7 +260,8 @@ main (int argc, char *argv[])
case 'f':
if (file) {
- fprintf (stderr, _("guestfish: only one -f parameter can be
given\n"));
+ fprintf (stderr, _("%s: only one -f parameter can be given\n"),
+ program_name);
exit (1);
}
file = optarg;
@@ -287,21 +311,20 @@ main (int argc, char *argv[])
break;
case 'V':
- printf ("guestfish %s\n", PACKAGE_VERSION);
+ printf ("%s %s\n", program_name, PACKAGE_VERSION);
exit (0);
case 'x':
echo_commands = 1;
break;
- case '?':
- usage ();
- exit (0);
+ case HELP_OPTION:
+ usage (0);
default:
- fprintf (stderr, _("guestfish: unexpected command line option 0x%x\n"),
- c);
- exit (1);
+ fprintf (stderr, _("%s: unexpected command line option 0x%x\n"),
+ program_name, c);
+ usage (1);
}
}
@@ -312,19 +335,25 @@ main (int argc, char *argv[])
if (drvs || mps || remote_control_listen || remote_control ||
guestfs_get_selinux (g)) {
- fprintf (stderr, _("guestfish: cannot use -i option with -a, -m, --listen,
--remote or --selinux\n"));
+ fprintf (stderr, _("%s: cannot use -i option with -a, -m,"
+ " --listen, --remote or --selinux\n"),
+ program_name);
exit (1);
}
if (optind >= argc) {
- fprintf (stderr, _("guestfish -i requires a libvirt domain or path(s) to disk
image(s)\n"));
+ fprintf (stderr,
+ _("%s: -i requires a libvirt domain or path(s) to disk
image(s)\n"),
+ program_name);
exit (1);
}
strcpy (cmd, "a=`virt-inspector");
while (optind < argc) {
- if (strlen (cmd) + strlen (argv[optind]) + strlen (argv[0]) + 60
+ if (strlen (cmd) + strlen (argv[optind]) + strlen (real_argv0) + 60
= sizeof cmd) {
- fprintf (stderr, _("guestfish:
virt-inspector command too long for fixed-size buffer\n"));
+ fprintf (stderr,
+ _("%s: virt-inspector command too long for fixed-size
buffer\n"),
+ program_name);
exit (1);
}
strcat (cmd, " '");
@@ -338,7 +367,7 @@ main (int argc, char *argv[])
else
strcat (cmd, " --fish");
- sprintf (&cmd[strlen(cmd)], "` && %s $a", argv[0]);
+ sprintf (&cmd[strlen(cmd)], "` && %s $a", real_argv0);
if (guestfs_get_verbose (g))
strcat (cmd, " -v");
@@ -347,7 +376,7 @@ main (int argc, char *argv[])
if (verbose)
fprintf (stderr,
- "guestfish -i: running virt-inspector command:\n%s\n", cmd);
+ "%s -i: running virt-inspector command:\n%s\n", program_name,
cmd);
r = system (cmd);
if (r == -1) {
@@ -368,17 +397,23 @@ main (int argc, char *argv[])
/* Remote control? */
if (remote_control_listen && remote_control) {
- fprintf (stderr, _("guestfish: cannot use --listen and --remote options at the
same time\n"));
+ fprintf (stderr,
+ _("%s: cannot use --listen and --remote options at the same
time\n"),
+ program_name);
exit (1);
}
if (remote_control_listen) {
if (optind < argc) {
- fprintf (stderr, _("guestfish: extra parameters on the command line with
--listen flag\n"));
+ fprintf (stderr,
+ _("%s: extra parameters on the command line with --listen
flag\n"),
+ program_name);
exit (1);
}
if (file) {
- fprintf (stderr, _("guestfish: cannot use --listen and --file options at the
same time\n"));
+ fprintf (stderr,
+ _("%s: cannot use --listen and --file options at the same
time\n"),
+ program_name);
exit (1);
}
rc_listen ();
@@ -602,12 +637,14 @@ script (int prompt)
p++;
len = strcspn (p, "\"");
if (p[len] == '\0') {
- fprintf (stderr, _("guestfish: unterminated double quote\n"));
+ fprintf (stderr, _("%s: unterminated double quote\n"),
program_name);
if (exit_on_error) exit (1);
goto next_command;
}
if (p[len+1] && (p[len+1] != ' ' && p[len+1] !=
'\t')) {
- fprintf (stderr, _("guestfish: command arguments not separated by
whitespace\n"));
+ fprintf (stderr,
+ _("%s: command arguments not separated by whitespace\n"),
+ program_name);
if (exit_on_error) exit (1);
goto next_command;
}
@@ -617,12 +654,14 @@ script (int prompt)
p++;
len = strcspn (p, "'");
if (p[len] == '\0') {
- fprintf (stderr, _("guestfish: unterminated single quote\n"));
+ fprintf (stderr, _("%s: unterminated single quote\n"),
program_name);
if (exit_on_error) exit (1);
goto next_command;
}
if (p[len+1] && (p[len+1] != ' ' && p[len+1] !=
'\t')) {
- fprintf (stderr, _("guestfish: command arguments not separated by
whitespace\n"));
+ fprintf (stderr,
+ _("%s: command arguments not separated by whitespace\n"),
+ program_name);
if (exit_on_error) exit (1);
goto next_command;
}
@@ -643,12 +682,15 @@ script (int prompt)
pend++;
}
if (c != 0) {
- fprintf (stderr, _("guestfish: unterminated \"[...]\"
sequence\n"));
+ fprintf (stderr,
+ _("%s: unterminated \"[...]\" sequence\n"),
program_name);
if (exit_on_error) exit (1);
goto next_command;
}
if (*pend && (*pend != ' ' && *pend != '\t')) {
- fprintf (stderr, _("guestfish: command arguments not separated by
whitespace\n"));
+ fprintf (stderr,
+ _("%s: command arguments not separated by whitespace\n"),
+ program_name);
if (exit_on_error) exit (1);
goto next_command;
}
@@ -667,8 +709,8 @@ script (int prompt)
} else
pend = &p[len];
} else {
- fprintf (stderr, _("guestfish: internal error parsing string at
'%s'\n"),
- p);
+ fprintf (stderr, _("%s: internal error parsing string at
'%s'\n"),
+ program_name, p);
abort ();
}
@@ -684,7 +726,7 @@ script (int prompt)
}
if (i == sizeof argv / sizeof argv[0]) {
- fprintf (stderr, _("guestfish: too many arguments\n"));
+ fprintf (stderr, _("%s: too many arguments\n"), program_name);
if (exit_on_error) exit (1);
goto next_command;
}
@@ -713,7 +755,7 @@ cmdline (char *argv[], int optind, int argc)
cmd = argv[optind++];
if (strcmp (cmd, ":") == 0) {
- fprintf (stderr, _("guestfish: empty command on command line\n"));
+ fprintf (stderr, _("%s: empty command on command line\n"), program_name);
exit (1);
}
params = &argv[optind];
--
1.6.4.378.g88f2f
11:04 a.m.
New subject: [PATCH libguestfs 2/5] guestfish: write --help to stdout, use gnulib's progname module
On Mon, Aug 24, 2009 at 06:00:19PM +0200, Jim Meyering wrote:
@@ -186,6 +201,11 @@ main (int argc, char *argv[])
(argv[0][0] != '/' || strstr (argv[0], "/.libs/lt-") != NULL))
guestfs_set_path (g, "appliance:" GUESTFS_DEFAULT_PATH);
+ /* getopt_long uses argv[0], so give it the sanitized name, too.
+ But only temporarily. */
+ char *real_argv0 = argv[0];
+ argv[0] = bad_cast (program_name);
ACK. Do you think you could change that comment to make it really
clear that this piece of code mustn't ever move above the path munging
done in the preceeding lines.
Thanks,
Rich.
--
Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine. Supports Linux and Windows.
http://et.redhat.com/~rjones/virt-df/
11:16 a.m.
New subject: [PATCH libguestfs 2/5] guestfish: write --help to stdout, use gnulib's progname module
Richard W.M. Jones wrote:
On Mon, Aug 24, 2009 at 06:00:19PM +0200, Jim Meyering wrote:
> @@ -186,6 +201,11 @@ main (int argc, char *argv[])
> (argv[0][0] != '/' || strstr (argv[0], "/.libs/lt-") !=
NULL))
> guestfs_set_path (g, "appliance:" GUESTFS_DEFAULT_PATH);
>
> + /* getopt_long uses argv[0], so give it the sanitized name, too.
> + But only temporarily. */
> + char *real_argv0 = argv[0];
> + argv[0] = bad_cast (program_name);
ACK. Do you think you could change that comment to make it really
clear that this piece of code mustn't ever move above the path munging
done in the preceeding lines.
Sure. Here's the incremental:
diff --git a/fish/fish.c b/fish/fish.c
index e799f86..91c5dad 100644
--- a/fish/fish.c
+++ b/fish/fish.c
@@ -201,8 +201,12 @@ main (int argc, char *argv[])
(argv[0][0] != '/' || strstr (argv[0], "/.libs/lt-") != NULL))
guestfs_set_path (g, "appliance:" GUESTFS_DEFAULT_PATH);
- /* getopt_long uses argv[0], so give it the sanitized name, too.
- But only temporarily. */
+ /* CAUTION: we are careful to modify argv[0] here, only after
+ * using it just above.
+ *
+ * getopt_long uses argv[0], so give it the sanitized name. Save a copy
+ * of the original, in case it's needed in virt-inspector mode, below.
+ */
char *real_argv0 = argv[0];
argv[0] = bad_cast (program_name);
11:33 a.m.
New subject: [PATCH libguestfs 2/5] guestfish: write --help to stdout, use gnulib's progname module
On Mon, Aug 24, 2009 at 06:16:06PM +0200, Jim Meyering wrote:
Richard W.M. Jones wrote:
> On Mon, Aug 24, 2009 at 06:00:19PM +0200, Jim Meyering wrote:
>> @@ -186,6 +201,11 @@ main (int argc, char *argv[])
>> (argv[0][0] != '/' || strstr (argv[0], "/.libs/lt-") !=
NULL))
>> guestfs_set_path (g, "appliance:" GUESTFS_DEFAULT_PATH);
>>
>> + /* getopt_long uses argv[0], so give it the sanitized name, too.
>> + But only temporarily. */
>> + char *real_argv0 = argv[0];
>> + argv[0] = bad_cast (program_name);
>
> ACK. Do you think you could change that comment to make it really
> clear that this piece of code mustn't ever move above the path munging
> done in the preceeding lines.
Sure. Here's the incremental:
diff --git a/fish/fish.c b/fish/fish.c
index e799f86..91c5dad 100644
--- a/fish/fish.c
+++ b/fish/fish.c
@@ -201,8 +201,12 @@ main (int argc, char *argv[])
(argv[0][0] != '/' || strstr (argv[0], "/.libs/lt-") != NULL))
guestfs_set_path (g, "appliance:" GUESTFS_DEFAULT_PATH);
- /* getopt_long uses argv[0], so give it the sanitized name, too.
- But only temporarily. */
+ /* CAUTION: we are careful to modify argv[0] here, only after
+ * using it just above.
+ *
+ * getopt_long uses argv[0], so give it the sanitized name. Save a copy
+ * of the original, in case it's needed in virt-inspector mode, below.
+ */
char *real_argv0 = argv[0];
argv[0] = bad_cast (program_name);
Yes, this comment is a great improvement I think.
Rich.
--
Richard Jones, Emerging Technologies, Red Hat http://et.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
5993
days inactive
5993
days old
27 comments
2 participants
participants (2)
-
Jim Meyering -
Richard W.M. Jones