[libvirt] [PATCH] libvirtd: mark strings for translation, including --help output

Saw these while fixing the previous bug:
From ab3d0ba4f5c004a45da5b5a240a0b5caef21df99 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering@redhat.com> Date: Thu, 20 May 2010 10:01:32 +0200 Subject: [PATCH] libvirtd: mark strings for translation, including --help output
* daemon/libvirtd.c (daemonForkIntoBackground, main): Mark strings for translation. (usage): Rework --help so that it is translatable, replacing each embedded, configuration-dependent, macro with an `%s'. libvirtd: don't ignore virInitialize failure * daemon/libvirtd.c (main): Diagnose virInitialize failure and exit nonzero. --- daemon/libvirtd.c | 36 +++++++++++++++++++++--------------- 1 files changed, 21 insertions(+), 15 deletions(-) diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index aac2d08..d52b6eb 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -484,8 +484,8 @@ static int daemonForkIntoBackground(void) { if (ret == 1 && status != 0) { fprintf(stderr, - "error: %s. Check /var/log/messages or run without " - "--daemon for more info.\n", + _("error: %s. Check /var/log/messages or run without " + "--daemon for more info.\n"), virDaemonErrTypeToString(status)); } _exit(ret == 1 && status == 0 ? 0 : 1); @@ -2963,7 +2963,7 @@ static void usage (const char *argv0) { fprintf (stderr, - "\n\ + _("\n\ Usage:\n\ %s [options]\n\ \n\ @@ -2981,27 +2981,33 @@ libvirt management daemon:\n\ Default paths:\n\ \n\ Configuration file (unless overridden by -f):\n\ - " SYSCONF_DIR "/libvirt/libvirtd.conf\n\ + %s/libvirt/libvirtd.conf\n\ \n\ Sockets (as root):\n\ - " LOCAL_STATE_DIR "/run/libvirt/libvirt-sock\n\ - " LOCAL_STATE_DIR "/run/libvirt/libvirt-sock-ro\n\ + %s/run/libvirt/libvirt-sock\n\ + %s/run/libvirt/libvirt-sock-ro\n\ \n\ Sockets (as non-root):\n\ $HOME/.libvirt/libvirt-sock (in UNIX abstract namespace)\n\ \n\ TLS:\n\ - CA certificate: " LIBVIRT_CACERT "\n\ - Server certificate: " LIBVIRT_SERVERCERT "\n\ - Server private key: " LIBVIRT_SERVERKEY "\n\ + CA certificate: %s\n\ + Server certificate: %s\n\ + Server private key: %s\n\ \n\ PID file (unless overridden by --pid-file):\n\ %s\n\ -\n", - argv0, - REMOTE_PID_FILE[0] != '\0' - ? REMOTE_PID_FILE - : "(disabled in ./configure)"); +\n"), + argv0, + SYSCONF_DIR, + LOCAL_STATE_DIR, + LOCAL_STATE_DIR, + LIBVIRT_CACERT, + LIBVIRT_SERVERCERT, + LIBVIRT_SERVERKEY, + (REMOTE_PID_FILE[0] != '\0' + ? REMOTE_PID_FILE + : "(disabled in ./configure)")); } enum { @@ -3083,7 +3089,7 @@ int main(int argc, char **argv) { return 2; default: - fprintf (stderr, "libvirtd: internal error: unknown flag: %c\n", + fprintf (stderr, _("libvirtd: internal error: unknown flag: %c\n"), c); exit (EXIT_FAILURE); } -- 1.7.1.259.g3aef8

On 05/20/2010 02:02 AM, Jim Meyering wrote:
+++ b/daemon/libvirtd.c @@ -484,8 +484,8 @@ static int daemonForkIntoBackground(void) {
if (ret == 1 && status != 0) { fprintf(stderr, - "error: %s. Check /var/log/messages or run without " - "--daemon for more info.\n", + _("error: %s. Check /var/log/messages or run without " + "--daemon for more info.\n"),
Should we also do a followup that passes argv[0] to this method, so that the error message can start with the program name?
virDaemonErrTypeToString(status)); } _exit(ret == 1 && status == 0 ? 0 : 1); @@ -2963,7 +2963,7 @@ static void usage (const char *argv0) { fprintf (stderr, - "\n\ + _("\n\ Usage:\n\ %s [options]\n\
As is the case in usage()?
+ (REMOTE_PID_FILE[0] != '\0' + ? REMOTE_PID_FILE + : "(disabled in ./configure)"));
Missed a string. This should be _("(disabled in ./configure)").
default: - fprintf (stderr, "libvirtd: internal error: unknown flag: %c\n", + fprintf (stderr, _("libvirtd: internal error: unknown flag: %c\n"),
And here, should we be using %s/argv[0] instead of hard-coding the name "libvirtd"? -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

Eric Blake wrote:
On 05/20/2010 02:02 AM, Jim Meyering wrote:
+++ b/daemon/libvirtd.c @@ -484,8 +484,8 @@ static int daemonForkIntoBackground(void) {
if (ret == 1 && status != 0) { fprintf(stderr, - "error: %s. Check /var/log/messages or run without " - "--daemon for more info.\n", + _("error: %s. Check /var/log/messages or run without " + "--daemon for more info.\n"),
Should we also do a followup that passes argv[0] to this method, so that the error message can start with the program name?
Might as well. patch below.
virDaemonErrTypeToString(status)); } _exit(ret == 1 && status == 0 ? 0 : 1); @@ -2963,7 +2963,7 @@ static void usage (const char *argv0) { fprintf (stderr, - "\n\ + _("\n\ Usage:\n\ %s [options]\n\
As is the case in usage()?
+ (REMOTE_PID_FILE[0] != '\0' + ? REMOTE_PID_FILE + : "(disabled in ./configure)"));
Missed a string. This should be _("(disabled in ./configure)").
Thanks. Fixed in the v2 below.
default: - fprintf (stderr, "libvirtd: internal error: unknown flag: %c\n", + fprintf (stderr, _("libvirtd: internal error: unknown flag: %c\n"),
And here, should we be using %s/argv[0] instead of hard-coding the name "libvirtd"?
Yes.
From c5df9b5d7e29234b43d07f998971e4f61e24d0f1 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering@redhat.com> Date: Thu, 20 May 2010 10:01:32 +0200 Subject: [PATCH v2 1/2] libvirtd: mark strings for translation, including --help output
* daemon/libvirtd.c (daemonForkIntoBackground, main): Mark strings for translation. (usage): Rework --help so that it is translatable, replacing each embedded, configuration-dependent, macro with an `%s'. libvirtd: don't ignore virInitialize failure * daemon/libvirtd.c (main): Diagnose virInitialize failure and exit nonzero. --- daemon/libvirtd.c | 36 +++++++++++++++++++++--------------- 1 files changed, 21 insertions(+), 15 deletions(-) diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index aac2d08..be28165 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -484,8 +484,8 @@ static int daemonForkIntoBackground(void) { if (ret == 1 && status != 0) { fprintf(stderr, - "error: %s. Check /var/log/messages or run without " - "--daemon for more info.\n", + _("error: %s. Check /var/log/messages or run without " + "--daemon for more info.\n"), virDaemonErrTypeToString(status)); } _exit(ret == 1 && status == 0 ? 0 : 1); @@ -2963,7 +2963,7 @@ static void usage (const char *argv0) { fprintf (stderr, - "\n\ + _("\n\ Usage:\n\ %s [options]\n\ \n\ @@ -2981,27 +2981,33 @@ libvirt management daemon:\n\ Default paths:\n\ \n\ Configuration file (unless overridden by -f):\n\ - " SYSCONF_DIR "/libvirt/libvirtd.conf\n\ + %s/libvirt/libvirtd.conf\n\ \n\ Sockets (as root):\n\ - " LOCAL_STATE_DIR "/run/libvirt/libvirt-sock\n\ - " LOCAL_STATE_DIR "/run/libvirt/libvirt-sock-ro\n\ + %s/run/libvirt/libvirt-sock\n\ + %s/run/libvirt/libvirt-sock-ro\n\ \n\ Sockets (as non-root):\n\ $HOME/.libvirt/libvirt-sock (in UNIX abstract namespace)\n\ \n\ TLS:\n\ - CA certificate: " LIBVIRT_CACERT "\n\ - Server certificate: " LIBVIRT_SERVERCERT "\n\ - Server private key: " LIBVIRT_SERVERKEY "\n\ + CA certificate: %s\n\ + Server certificate: %s\n\ + Server private key: %s\n\ \n\ PID file (unless overridden by --pid-file):\n\ %s\n\ -\n", - argv0, - REMOTE_PID_FILE[0] != '\0' - ? REMOTE_PID_FILE - : "(disabled in ./configure)"); +\n"), + argv0, + SYSCONF_DIR, + LOCAL_STATE_DIR, + LOCAL_STATE_DIR, + LIBVIRT_CACERT, + LIBVIRT_SERVERCERT, + LIBVIRT_SERVERKEY, + (REMOTE_PID_FILE[0] != '\0' + ? REMOTE_PID_FILE + : _("(disabled in ./configure)"))); } enum { @@ -3083,7 +3089,7 @@ int main(int argc, char **argv) { return 2; default: - fprintf (stderr, "libvirtd: internal error: unknown flag: %c\n", + fprintf (stderr, _("libvirtd: internal error: unknown flag: %c\n"), c); exit (EXIT_FAILURE); } -- 1.7.1.262.g5ef3d
From 2405307cc444d0ecdd6aa2326f29aee4f2375b49 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering@redhat.com> Date: Thu, 20 May 2010 21:40:54 +0200 Subject: [PATCH 2/2] libvirtd: use argv[0] in place of hard-coded "libvirtd" in diagnostics
* daemon/libvirtd.c (main): Use argv[0] in place of hard-coded "libvirtd" in two diagnostics. Suggested by Eric Blake. --- daemon/libvirtd.c | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index be28165..f82e282 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -3035,7 +3035,7 @@ int main(int argc, char **argv) { }; if (virInitialize() < 0) { - fprintf (stderr, _("libvirtd: initialization failed\n")); + fprintf (stderr, _("%s: initialization failed\n"), argv[0]); exit (EXIT_FAILURE); } @@ -3089,8 +3089,8 @@ int main(int argc, char **argv) { return 2; default: - fprintf (stderr, _("libvirtd: internal error: unknown flag: %c\n"), - c); + fprintf (stderr, _("%s: internal error: unknown flag: %c\n"), + argv[0], c); exit (EXIT_FAILURE); } } -- 1.7.1.262.g5ef3d

On 05/20/2010 01:42 PM, Jim Meyering wrote:
Should we also do a followup that passes argv[0] to this method, so that the error message can start with the program name?
Might as well. patch below.
Didn't see any signature change to daemonForkIntoBackground...
Missed a string. This should be _("(disabled in ./configure)").
Thanks. Fixed in the v2 below.
Confirmed.
default: - fprintf (stderr, "libvirtd: internal error: unknown flag: %c\n", + fprintf (stderr, _("libvirtd: internal error: unknown flag: %c\n"),
And here, should we be using %s/argv[0] instead of hard-coding the name "libvirtd"?
Yes.
ACK to 1/2, but let's respin 2/2 to tweak daemonForkIntoBackground to take another parameter... -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

Eric Blake wrote:
On 05/20/2010 01:42 PM, Jim Meyering wrote:
Should we also do a followup that passes argv[0] to this method, so that the error message can start with the program name?
Might as well. patch below.
Didn't see any signature change to daemonForkIntoBackground...
Missed a string. This should be _("(disabled in ./configure)").
Thanks. Fixed in the v2 below.
Confirmed.
default: - fprintf (stderr, "libvirtd: internal error: unknown flag: %c\n", + fprintf (stderr, _("libvirtd: internal error: unknown flag: %c\n"),
And here, should we be using %s/argv[0] instead of hard-coding the name "libvirtd"?
Yes.
ACK to 1/2, but let's respin 2/2 to tweak daemonForkIntoBackground to take another parameter...
Um... Well there are two other diagnostics with no progname: prefix. They're in qemudWritePidFile: VIR_ERROR(_("Failed to write to pid file '%s' : %s") VIR_ERROR(_("Failed to close pid file '%s' : %s") so rather than adding parameters to two functions, I'll add a global, argv0, and *remove* a few parameters:
From 3dde80f86d7a6afe59d5ae9c0dff70a592cf0c84 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering@redhat.com> Date: Thu, 20 May 2010 21:40:54 +0200 Subject: [PATCH] libvirtd: start each diagnostic with "argv0: "
Some diagnostics had a hard-coded "libvirtd: " prefix, some used "error: " and some used "argv[0]: ". Always use "argv[0]: ". * daemon/libvirtd.c (argv0): New global. (main): Set it. (version, usage): Remove argv0 parameter. Use global; update callers. (daemonForkIntoBackground): Use argv0:, not error:. (qemudWritePidFile): Start each diagnostic with argv0:. Suggested by Eric Blake. --- daemon/libvirtd.c | 28 +++++++++++++++------------- 1 files changed, 15 insertions(+), 13 deletions(-) diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index be28165..195c50a 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -190,6 +190,7 @@ static int max_client_requests = 5; static sig_atomic_t sig_errors = 0; static int sig_lasterrno = 0; +static const char *argv0; enum { VIR_DAEMON_ERR_NONE = 0, @@ -484,8 +485,8 @@ static int daemonForkIntoBackground(void) { if (ret == 1 && status != 0) { fprintf(stderr, - _("error: %s. Check /var/log/messages or run without " - "--daemon for more info.\n"), + _("%s: error: %s. Check /var/log/messages or run without " + "--daemon for more info.\n"), argv0, virDaemonErrTypeToString(status)); } _exit(ret == 1 && status == 0 ? 0 : 1); @@ -515,15 +516,15 @@ static int qemudWritePidFile(const char *pidFile) { } if (fprintf(fh, "%lu\n", (unsigned long)getpid()) < 0) { - VIR_ERROR(_("Failed to write to pid file '%s' : %s"), - pidFile, virStrerror(errno, ebuf, sizeof ebuf)); + VIR_ERROR(_("%s: Failed to write to pid file '%s' : %s"), + argv0, pidFile, virStrerror(errno, ebuf, sizeof ebuf)); fclose(fh); return -1; } if (fclose(fh) == EOF) { - VIR_ERROR(_("Failed to close pid file '%s' : %s"), - pidFile, virStrerror(errno, ebuf, sizeof ebuf)); + VIR_ERROR(_("%s: Failed to close pid file '%s' : %s"), + argv0, pidFile, virStrerror(errno, ebuf, sizeof ebuf)); return -1; } @@ -2868,7 +2869,7 @@ remoteReadConfigFile (struct qemud_server *server, const char *filename) /* Display version information. */ static void -version (const char *argv0) +version (void) { printf ("%s (%s) %s\n", argv0, PACKAGE_NAME, PACKAGE_VERSION); } @@ -2960,7 +2961,7 @@ error: /* Print command-line usage. */ static void -usage (const char *argv0) +usage (void) { fprintf (stderr, _("\n\ @@ -3021,6 +3022,7 @@ int main(int argc, char **argv) { const char *remote_config_file = NULL; int statuswrite = -1; int ret = 1; + argv0 = argv[0]; struct option opts[] = { { "verbose", no_argument, &verbose, 1}, @@ -3035,7 +3037,7 @@ int main(int argc, char **argv) { }; if (virInitialize() < 0) { - fprintf (stderr, _("libvirtd: initialization failed\n")); + fprintf (stderr, _("%s: initialization failed\n"), argv0); exit (EXIT_FAILURE); } @@ -3081,16 +3083,16 @@ int main(int argc, char **argv) { break; case OPT_VERSION: - version (argv[0]); + version (); return 0; case '?': - usage (argv[0]); + usage (); return 2; default: - fprintf (stderr, _("libvirtd: internal error: unknown flag: %c\n"), - c); + fprintf (stderr, _("%s: internal error: unknown flag: %c\n"), + argv0, c); exit (EXIT_FAILURE); } } -- 1.7.1.262.g5ef3d

On 05/20/2010 02:49 PM, Jim Meyering wrote:
ACK to 1/2, but let's respin 2/2 to tweak daemonForkIntoBackground to take another parameter...
Um... Well there are two other diagnostics with no progname: prefix. They're in qemudWritePidFile:
VIR_ERROR(_("Failed to write to pid file '%s' : %s") VIR_ERROR(_("Failed to close pid file '%s' : %s")
so rather than adding parameters to two functions, I'll add a global, argv0, and *remove* a few parameters:
Adding the global is nice. I had to go look at VIR_ERROR (forwards to virLogMessage, which in turn prints a timestamp but does not print argv0) to make sure that it made sense to add argv0 there. So, ACK. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

Eric Blake wrote:
On 05/20/2010 02:49 PM, Jim Meyering wrote:
ACK to 1/2, but let's respin 2/2 to tweak daemonForkIntoBackground to take another parameter...
Um... Well there are two other diagnostics with no progname: prefix. They're in qemudWritePidFile:
VIR_ERROR(_("Failed to write to pid file '%s' : %s") VIR_ERROR(_("Failed to close pid file '%s' : %s")
so rather than adding parameters to two functions, I'll add a global, argv0, and *remove* a few parameters:
Adding the global is nice. I had to go look at VIR_ERROR (forwards to virLogMessage, which in turn prints a timestamp but does not print argv0) to make sure that it made sense to add argv0 there. So,
Thanks. Pushed.
participants (2)
-
Eric Blake
-
Jim Meyering