Take setlocale/gettext error handling pattern from tools/virsh-* and use it for all standalone binaries via a new shared virGettextInitialize routine. The virsh* pattern differed slightly from other callers. All users now consistently: * Ignore setlocale errors. virsh has done this forever, presumably for good reason. This has been partially responsible for some bug reports: https://bugzilla.redhat.com/show_bug.cgi?id=1312688 https://bugzilla.redhat.com/show_bug.cgi?id=1026514 https://bugzilla.redhat.com/show_bug.cgi?id=1016158 * Report the failed function name * Report strerror --- v3: Fix make syntax-check; add a rule similar to the bindtextdomain check that checks for the new virGettextInitialize cfg.mk | 11 +++++++++ daemon/libvirtd.c | 6 ++--- src/Makefile.am | 2 ++ src/libvirt_private.syms | 4 ++++ src/locking/lock_daemon.c | 6 ++--- src/locking/sanlock_helper.c | 9 ++----- src/logging/log_daemon.c | 6 ++--- src/lxc/lxc_controller.c | 6 ++--- src/network/leaseshelper.c | 12 +++------- src/security/virt-aa-helper.c | 12 +++------- src/storage/parthelper.c | 9 ++----- src/util/iohelper.c | 13 +++------- src/util/virgettext.c | 56 +++++++++++++++++++++++++++++++++++++++++++ src/util/virgettext.h | 25 +++++++++++++++++++ tools/virsh.c | 15 ++---------- tools/virt-admin.c | 15 ++---------- tools/virt-host-validate.c | 15 ++---------- tools/virt-login-shell.c | 14 ++--------- tools/vsh.c | 2 -- 19 files changed, 127 insertions(+), 111 deletions(-) create mode 100644 src/util/virgettext.c create mode 100644 src/util/virgettext.h diff --git a/cfg.mk b/cfg.mk index 5372584..90bb709 100644 --- a/cfg.mk +++ b/cfg.mk @@ -1037,6 +1037,15 @@ sc_prohibit_verbose_strcat: halt='Use strcat(a, b) instead of strncat(a, b, strlen(b))' \ $(_sc_search_regexp) +# Ensure that each .c file containing a "main" function also +# calls virGettextInitialize +sc_gettext_init: + @require='virGettextInitialize *\(' \ + in_vc_files='\.c$$' \ + containing='\<main *(' \ + halt='the above files do not call virGettextInitialize' \ + $(_sc_search_regexp) + # We don't use this feature of maint.mk. prev_version_file = /dev/null @@ -1133,6 +1142,8 @@ _test1=shunloadtest|virnettlscontexttest|virnettlssessiontest|vircgroupmock exclude_file_name_regexp--sc_avoid_write = \ ^(src/($(_src1))|daemon/libvirtd|tools/virsh-console|tests/($(_test1)))\.c$$ +exclude_file_name_regexp--sc_bindtextdomain = .* + exclude_file_name_regexp--sc_gettext_init = ^(tests|examples)/ exclude_file_name_regexp--sc_copyright_usage = \ diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 3d38a46..5f66e8b 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -30,7 +30,6 @@ #include <getopt.h> #include <stdlib.h> #include <grp.h> -#include <locale.h> #include "libvirt_internal.h" #include "virerror.h" @@ -58,6 +57,7 @@ #include "locking/lock_manager.h" #include "viraccessmanager.h" #include "virutil.h" +#include "virgettext.h" #ifdef WITH_DRIVER_MODULES # include "driver.h" @@ -1172,9 +1172,7 @@ int main(int argc, char **argv) { {0, 0, 0, 0} }; - if (setlocale(LC_ALL, "") == NULL || - bindtextdomain(PACKAGE, LOCALEDIR) == NULL || - textdomain(PACKAGE) == NULL || + if (virGettextInitialize() < 0 || virInitialize() < 0) { fprintf(stderr, _("%s: initialization failed\n"), argv[0]); exit(EXIT_FAILURE); diff --git a/src/Makefile.am b/src/Makefile.am index eda0365..38b9560 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -114,6 +114,7 @@ UTIL_SOURCES = \ util/virfile.c util/virfile.h \ util/virfirewall.c util/virfirewall.h \ util/virfirewallpriv.h \ + util/virgettext.c util/virgettext.h \ util/virgic.c util/virgic.h \ util/virhash.c util/virhash.h \ util/virhashcode.c util/virhashcode.h \ @@ -2321,6 +2322,7 @@ libvirt_setuid_rpc_client_la_SOURCES = \ util/virevent.c \ util/vireventpoll.c \ util/virfile.c \ + util/virgettext.c \ util/virhash.c \ util/virhashcode.c \ util/virjson.c \ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 068bc00..af7de8a 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1517,6 +1517,10 @@ virFirewallStartRollback; virFirewallStartTransaction; +# util/virgettext.h +virGettextInitialize; + + # util/virgic.h virGICVersionTypeFromString; virGICVersionTypeToString; diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c index 973e691..bfdcfc6 100644 --- a/src/locking/lock_daemon.c +++ b/src/locking/lock_daemon.c @@ -28,7 +28,6 @@ #include <sys/stat.h> #include <getopt.h> #include <stdlib.h> -#include <locale.h> #include "lock_daemon.h" @@ -47,6 +46,7 @@ #include "virhash.h" #include "viruuid.h" #include "virstring.h" +#include "virgettext.h" #include "locking/lock_daemon_dispatch.h" #include "locking/lock_protocol.h" @@ -1179,9 +1179,7 @@ int main(int argc, char **argv) { privileged = geteuid() == 0; - if (setlocale(LC_ALL, "") == NULL || - bindtextdomain(PACKAGE, LOCALEDIR) == NULL || - textdomain(PACKAGE) == NULL || + if (virGettextInitialize() < 0 || virThreadInitialize() < 0 || virErrorInitialize() < 0) { fprintf(stderr, _("%s: initialization failed\n"), argv[0]); diff --git a/src/locking/sanlock_helper.c b/src/locking/sanlock_helper.c index d8d294f..57e1cfb 100644 --- a/src/locking/sanlock_helper.c +++ b/src/locking/sanlock_helper.c @@ -1,13 +1,12 @@ #include <config.h> #include <stdio.h> #include <stdlib.h> -#include <locale.h> -#include "configmake.h" #include "internal.h" #include "virconf.h" #include "viralloc.h" #include "domain_conf.h" +#include "virgettext.h" static int @@ -70,12 +69,8 @@ main(int argc, char **argv) .cb = authCallback, }; - if (setlocale(LC_ALL, "") == NULL || - bindtextdomain(PACKAGE, LOCALEDIR) == NULL || - textdomain(PACKAGE) == NULL) { - fprintf(stderr, _("%s: initialization failed\n"), argv[0]); + if (virGettextInitialize() < 0) exit(EXIT_FAILURE); - } if (getArgs(argc, argv, &uri, &uuid, &action) < 0) goto cleanup; diff --git a/src/logging/log_daemon.c b/src/logging/log_daemon.c index f674cbd..70339af 100644 --- a/src/logging/log_daemon.c +++ b/src/logging/log_daemon.c @@ -28,7 +28,6 @@ #include <sys/stat.h> #include <getopt.h> #include <stdlib.h> -#include <locale.h> #include "log_daemon.h" @@ -46,6 +45,7 @@ #include "virhash.h" #include "viruuid.h" #include "virstring.h" +#include "virgettext.h" #include "log_daemon_dispatch.h" #include "log_protocol.h" @@ -936,9 +936,7 @@ int main(int argc, char **argv) { privileged = geteuid() == 0; - if (setlocale(LC_ALL, "") == NULL || - bindtextdomain(PACKAGE, LOCALEDIR) == NULL || - textdomain(PACKAGE) == NULL || + if (virGettextInitialize() < 0 || virThreadInitialize() < 0 || virErrorInitialize() < 0) { fprintf(stderr, _("%s: initialization failed\n"), argv[0]); diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index 8b5ec4c..73e57e3 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -36,7 +36,6 @@ #include <signal.h> #include <getopt.h> #include <sys/mount.h> -#include <locale.h> #include <grp.h> #include <sys/stat.h> #include <time.h> @@ -66,6 +65,7 @@ #include "virdbus.h" #include "rpc/virnetdaemon.h" #include "virstring.h" +#include "virgettext.h" #define VIR_FROM_THIS VIR_FROM_LXC @@ -2505,9 +2505,7 @@ int main(int argc, char *argv[]) for (i = 0; i < VIR_LXC_DOMAIN_NAMESPACE_LAST; i++) ns_fd[i] = -1; - if (setlocale(LC_ALL, "") == NULL || - bindtextdomain(PACKAGE, LOCALEDIR) == NULL || - textdomain(PACKAGE) == NULL || + if (virGettextInitialize() < 0 || virThreadInitialize() < 0 || virErrorInitialize() < 0) { fprintf(stderr, _("%s: initialization failed\n"), argv[0]); diff --git a/src/network/leaseshelper.c b/src/network/leaseshelper.c index 097cd11..16f6eb8 100644 --- a/src/network/leaseshelper.c +++ b/src/network/leaseshelper.c @@ -25,7 +25,6 @@ #include <config.h> -#include <locale.h> #include <stdio.h> #include <stdlib.h> @@ -38,6 +37,7 @@ #include "virjson.h" #include "virlease.h" #include "configmake.h" +#include "virgettext.h" #define VIR_FROM_THIS VIR_FROM_NETWORK @@ -115,14 +115,8 @@ main(int argc, char **argv) program_name = argv[0]; - if (setlocale(LC_ALL, "") == NULL || - bindtextdomain(PACKAGE, LOCALEDIR) == NULL || - textdomain(PACKAGE) == NULL) { - fprintf(stderr, _("%s: initialization failed\n"), program_name); - exit(EXIT_FAILURE); - } - - if (virThreadInitialize() < 0 || + if (virGettextInitialize() < 0 || + virThreadInitialize() < 0 || virErrorInitialize() < 0) { fprintf(stderr, _("%s: initialization failed\n"), program_name); exit(EXIT_FAILURE); diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 50d2a08..5db9c02 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -35,7 +35,6 @@ #include <fcntl.h> #include <getopt.h> #include <sys/utsname.h> -#include <locale.h> #include "internal.h" #include "virbuffer.h" @@ -54,6 +53,7 @@ #include "configmake.h" #include "virrandom.h" #include "virstring.h" +#include "virgettext.h" #include "storage/storage_driver.h" @@ -1298,14 +1298,8 @@ main(int argc, char **argv) char *profile = NULL; char *include_file = NULL; - if (setlocale(LC_ALL, "") == NULL || - bindtextdomain(PACKAGE, LOCALEDIR) == NULL || - textdomain(PACKAGE) == NULL) { - fprintf(stderr, _("%s: initialization failed\n"), argv[0]); - exit(EXIT_FAILURE); - } - - if (virThreadInitialize() < 0 || + if (virGettextInitialize() < 0 || + virThreadInitialize() < 0 || virErrorInitialize() < 0) { fprintf(stderr, _("%s: initialization failed\n"), argv[0]); exit(EXIT_FAILURE); diff --git a/src/storage/parthelper.c b/src/storage/parthelper.c index d1df068..6695f23 100644 --- a/src/storage/parthelper.c +++ b/src/storage/parthelper.c @@ -39,13 +39,12 @@ #include <sys/types.h> #include <sys/stat.h> #include <unistd.h> -#include <locale.h> #include "virutil.h" #include "virfile.h" #include "c-ctype.h" -#include "configmake.h" #include "virstring.h" +#include "virgettext.h" /* we don't need to include the full internal.h just for this */ #define STREQ(a, b) (strcmp(a, b) == 0) @@ -72,12 +71,8 @@ int main(int argc, char **argv) const char *partsep; bool devmap_nopartsep = false; - if (setlocale(LC_ALL, "") == NULL || - bindtextdomain(PACKAGE, LOCALEDIR) == NULL || - textdomain(PACKAGE) == NULL) { - fprintf(stderr, _("%s: initialization failed\n"), argv[0]); + if (virGettextInitialize() < 0) exit(EXIT_FAILURE); - } if (argc == 3 && STREQ(argv[2], "-g")) { cmd = DISK_GEOMETRY; diff --git a/src/util/iohelper.c b/src/util/iohelper.c index 8a3c377..275f993 100644 --- a/src/util/iohelper.c +++ b/src/util/iohelper.c @@ -27,7 +27,6 @@ #include <config.h> -#include <locale.h> #include <unistd.h> #include <fcntl.h> #include <stdio.h> @@ -38,9 +37,9 @@ #include "virfile.h" #include "viralloc.h" #include "virerror.h" -#include "configmake.h" #include "virrandom.h" #include "virstring.h" +#include "virgettext.h" #define VIR_FROM_THIS VIR_FROM_STORAGE @@ -230,14 +229,8 @@ main(int argc, char **argv) program_name = argv[0]; - if (setlocale(LC_ALL, "") == NULL || - bindtextdomain(PACKAGE, LOCALEDIR) == NULL || - textdomain(PACKAGE) == NULL) { - fprintf(stderr, _("%s: initialization failed\n"), program_name); - exit(EXIT_FAILURE); - } - - if (virThreadInitialize() < 0 || + if (virGettextInitialize() < 0 || + virThreadInitialize() < 0 || virErrorInitialize() < 0) { fprintf(stderr, _("%s: initialization failed\n"), program_name); exit(EXIT_FAILURE); diff --git a/src/util/virgettext.c b/src/util/virgettext.c new file mode 100644 index 0000000..7e32043 --- /dev/null +++ b/src/util/virgettext.c @@ -0,0 +1,56 @@ +/* + * virgettext.c: gettext helper routines + * + * Copyright (C) 2016 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library 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 + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + */ + +#include <config.h> + +#include <locale.h> +#include <stdio.h> + +#include "configmake.h" +#include "internal.h" +#include "virgettext.h" + + +/** + * virGettextInit: + * + * Initialize standard gettext setup + * Returns -1 on fatal error + */ +int +virGettextInitialize(void) +{ + if (!setlocale(LC_ALL, "")) { + perror("setlocale"); + /* failure to setup locale is not fatal */ + } + + if (!bindtextdomain(PACKAGE, LOCALEDIR)) { + perror("bindtextdomain"); + return -1; + } + + if (!textdomain(PACKAGE)) { + perror("textdomain"); + return -1; + } + + return 0; +} diff --git a/src/util/virgettext.h b/src/util/virgettext.h new file mode 100644 index 0000000..4584d83 --- /dev/null +++ b/src/util/virgettext.h @@ -0,0 +1,25 @@ +/* + * virgettext.h: gettext helper routines + * + * Copyright (C) 2016 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library 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 + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + */ +#ifndef __VIR_GETTEXT_H__ +# define __VIR_GETTEXT_H__ + +int virGettextInitialize(void); + +#endif /* __VIR_GETTEXT_H__ */ diff --git a/tools/virsh.c b/tools/virsh.c index fe33839..f632405 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -34,7 +34,6 @@ #include <getopt.h> #include <sys/time.h> #include <fcntl.h> -#include <locale.h> #include <time.h> #include <limits.h> #include <sys/stat.h> @@ -53,12 +52,12 @@ #include <libvirt/libvirt-qemu.h> #include <libvirt/libvirt-lxc.h> #include "virfile.h" -#include "configmake.h" #include "virthread.h" #include "vircommand.h" #include "conf/domain_conf.h" #include "virtypedparam.h" #include "virstring.h" +#include "virgettext.h" #include "virsh-console.h" #include "virsh-domain.h" @@ -933,18 +932,8 @@ main(int argc, char **argv) progname++; ctl->progname = progname; - if (!setlocale(LC_ALL, "")) { - perror("setlocale"); - /* failure to setup locale is not fatal */ - } - if (!bindtextdomain(PACKAGE, LOCALEDIR)) { - perror("bindtextdomain"); - return EXIT_FAILURE; - } - if (!textdomain(PACKAGE)) { - perror("textdomain"); + if (virGettextInitialize() < 0) return EXIT_FAILURE; - } if (isatty(STDIN_FILENO)) { ctl->istty = true; diff --git a/tools/virt-admin.c b/tools/virt-admin.c index f0a49a3..195088b 100644 --- a/tools/virt-admin.c +++ b/tools/virt-admin.c @@ -25,20 +25,19 @@ #include <errno.h> #include <getopt.h> -#include <locale.h> #if WITH_READLINE # include <readline/readline.h> # include <readline/history.h> #endif -#include "configmake.h" #include "internal.h" #include "viralloc.h" #include "virerror.h" #include "virfile.h" #include "virstring.h" #include "virthread.h" +#include "virgettext.h" /* Gnulib doesn't guarantee SA_SIGINFO support. */ #ifndef SA_SIGINFO @@ -689,18 +688,8 @@ main(int argc, char **argv) progname++; ctl->progname = progname; - if (!setlocale(LC_ALL, "")) { - perror("setlocale"); - /* failure to setup locale is not fatal */ - } - if (!bindtextdomain(PACKAGE, LOCALEDIR)) { - perror("bindtextdomain"); - return EXIT_FAILURE; - } - if (!textdomain(PACKAGE)) { - perror("textdomain"); + if (virGettextInitialize() < 0) return EXIT_FAILURE; - } if (isatty(STDIN_FILENO)) { ctl->istty = true; diff --git a/tools/virt-host-validate.c b/tools/virt-host-validate.c index a8c2075..5b7fe9b 100644 --- a/tools/virt-host-validate.c +++ b/tools/virt-host-validate.c @@ -25,10 +25,9 @@ #include <stdlib.h> #include <gettext.h> #include <getopt.h> -#include <locale.h> #include "internal.h" -#include "configmake.h" +#include "virgettext.h" #include "virt-host-validate-common.h" #if WITH_QEMU @@ -80,18 +79,8 @@ main(int argc, char **argv) bool quiet = false; bool usedHvname = false; - if (!setlocale(LC_ALL, "")) { - perror("setlocale"); - /* failure to setup locale is not fatal */ - } - if (!bindtextdomain(PACKAGE, LOCALEDIR)) { - perror("bindtextdomain"); - return EXIT_FAILURE; - } - if (!textdomain(PACKAGE)) { - perror("textdomain"); + if (virGettextInitialize() < 0) return EXIT_FAILURE; - } while ((c = getopt_long(argc, argv, "hvq", argOptions, NULL)) != -1) { switch (c) { diff --git a/tools/virt-login-shell.c b/tools/virt-login-shell.c index ec759dc..8f87227 100644 --- a/tools/virt-login-shell.c +++ b/tools/virt-login-shell.c @@ -24,7 +24,6 @@ #include <errno.h> #include <fnmatch.h> #include <getopt.h> -#include <locale.h> #include <signal.h> #include <stdarg.h> #include <stdio.h> @@ -41,6 +40,7 @@ #include "virstring.h" #include "viralloc.h" #include "vircommand.h" +#include "virgettext.h" #define VIR_FROM_THIS VIR_FROM_NONE static const char *conf_file = SYSCONFDIR "/libvirt/virt-login-shell.conf"; @@ -207,18 +207,8 @@ main(int argc, char **argv) virSetErrorLogPriorityFunc(NULL); progname = argv[0]; - if (!setlocale(LC_ALL, "")) { - perror("setlocale"); - /* failure to setup locale is not fatal */ - } - if (!bindtextdomain(PACKAGE, LOCALEDIR)) { - perror("bindtextdomain"); - return ret; - } - if (!textdomain(PACKAGE)) { - perror("textdomain"); + if (virGettextInitialize() < 0) return ret; - } while ((arg = getopt_long(argc, argv, "hV", opt, &longindex)) != -1) { switch (arg) { diff --git a/tools/vsh.c b/tools/vsh.c index 6bdc082..f033c05 100644 --- a/tools/vsh.c +++ b/tools/vsh.c @@ -35,7 +35,6 @@ #include <sys/time.h> #include "c-ctype.h" #include <fcntl.h> -#include <locale.h> #include <time.h> #include <limits.h> #include <sys/stat.h> @@ -55,7 +54,6 @@ #include <libvirt/libvirt-qemu.h> #include <libvirt/libvirt-lxc.h> #include "virfile.h" -#include "configmake.h" #include "virthread.h" #include "vircommand.h" #include "conf/domain_conf.h" -- 2.7.3