diff mbox

Be consistent with setlocale error handling

Message ID 60d1ded742e0caa619edf656986d6f3d87043e0a.1460469648.git.crobinso@redhat.com
State New
Headers show

Commit Message

Cole Robinson April 12, 2016, 2 p.m. UTC 
Take setlocale/gettext error handling pattern from tools/virsh-*
and use it in all the other standalone binaries. The changes are

* 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
---
 daemon/libvirtd.c             | 20 ++++++++++++++++----
 src/locking/lock_daemon.c     | 20 ++++++++++++++++----
 src/locking/sanlock_helper.c  | 16 ++++++++++++----
 src/logging/log_daemon.c      | 20 ++++++++++++++++----
 src/lxc/lxc_controller.c      | 20 ++++++++++++++++----
 src/network/leaseshelper.c    | 16 ++++++++++++----
 src/security/virt-aa-helper.c | 16 ++++++++++++----
 src/storage/parthelper.c      | 16 ++++++++++++----
 src/util/iohelper.c           | 16 ++++++++++++----
 9 files changed, 124 insertions(+), 36 deletions(-)

-- 
2.7.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Comments

Cole Robinson April 12, 2016, 2:11 p.m. UTC | #1 
On 04/12/2016 10:10 AM, Daniel P. Berrange wrote:
> On Tue, Apr 12, 2016 at 10:00:48AM -0400, Cole Robinson wrote:

>> Take setlocale/gettext error handling pattern from tools/virsh-*

>> and use it in all the other standalone binaries. The changes are

>>

>> * 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

>> ---

>>  daemon/libvirtd.c             | 20 ++++++++++++++++----

>>  src/locking/lock_daemon.c     | 20 ++++++++++++++++----

>>  src/locking/sanlock_helper.c  | 16 ++++++++++++----

>>  src/logging/log_daemon.c      | 20 ++++++++++++++++----

>>  src/lxc/lxc_controller.c      | 20 ++++++++++++++++----

>>  src/network/leaseshelper.c    | 16 ++++++++++++----

>>  src/security/virt-aa-helper.c | 16 ++++++++++++----

>>  src/storage/parthelper.c      | 16 ++++++++++++----

>>  src/util/iohelper.c           | 16 ++++++++++++----

>>  9 files changed, 124 insertions(+), 36 deletions(-)

>>

>> diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c

>> index 3d38a46..9488950 100644

>> --- a/daemon/libvirtd.c

>> +++ b/daemon/libvirtd.c

>> @@ -1172,10 +1172,22 @@ int main(int argc, char **argv) {

>>          {0, 0, 0, 0}

>>      };

>>  

>> -    if (setlocale(LC_ALL, "") == NULL ||

>> -        bindtextdomain(PACKAGE, LOCALEDIR) == NULL ||

>> -        textdomain(PACKAGE) == NULL ||

>> -        virInitialize() < 0) {

>> +    if (!setlocale(LC_ALL, "")) {

>> +        perror("setlocale");

>> +        /* failure to setup locale is not fatal */

>> +    }

>> +

>> +    if (!bindtextdomain(PACKAGE, LOCALEDIR)) {

>> +        perror("bindtextdomain");

>> +        exit(EXIT_FAILURE);

>> +    }

>> +

>> +    if (!textdomain(PACKAGE)) {

>> +        perror("textdomain");

>> +        exit(EXIT_FAILURE);

>> +    }

>> +

>> +    if (virInitialize() < 0) {

>>          fprintf(stderr, _("%s: initialization failed\n"), argv[0]);

>>          exit(EXIT_FAILURE);

>>      }

> 

> Instead of repeating this, how about we add  src/util/virgettext.h and

> then have

> 

> 

>   int virGettextInit(const char *package, const char *localedir) {

>     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;

>   }

> 

> 

> And in each app we can just do

> 

>    if (virGettextInit(PACKAGE, LOCALEDIR) < 0)

>       exit(EXIT_FAILURE);

> 

> Regards,

> Daniel

> 


Good idea, I'll send a v2

Thanks,
Cole

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
diff mbox

Patch

diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
index 3d38a46..9488950 100644
--- a/daemon/libvirtd.c
+++ b/daemon/libvirtd.c
@@ -1172,10 +1172,22 @@  int main(int argc, char **argv) {
         {0, 0, 0, 0}
     };
 
-    if (setlocale(LC_ALL, "") == NULL ||
-        bindtextdomain(PACKAGE, LOCALEDIR) == NULL ||
-        textdomain(PACKAGE) == NULL ||
-        virInitialize() < 0) {
+    if (!setlocale(LC_ALL, "")) {
+        perror("setlocale");
+        /* failure to setup locale is not fatal */
+    }
+
+    if (!bindtextdomain(PACKAGE, LOCALEDIR)) {
+        perror("bindtextdomain");
+        exit(EXIT_FAILURE);
+    }
+
+    if (!textdomain(PACKAGE)) {
+        perror("textdomain");
+        exit(EXIT_FAILURE);
+    }
+
+    if (virInitialize() < 0) {
         fprintf(stderr, _("%s: initialization failed\n"), argv[0]);
         exit(EXIT_FAILURE);
     }
diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c
index 973e691..fffbe1d 100644
--- a/src/locking/lock_daemon.c
+++ b/src/locking/lock_daemon.c
@@ -1179,10 +1179,22 @@  int main(int argc, char **argv) {
 
     privileged = geteuid() == 0;
 
-    if (setlocale(LC_ALL, "") == NULL ||
-        bindtextdomain(PACKAGE, LOCALEDIR) == NULL ||
-        textdomain(PACKAGE) == NULL ||
-        virThreadInitialize() < 0 ||
+    if (!setlocale(LC_ALL, "")) {
+        perror("setlocale");
+        /* failure to setup locale is not fatal */
+    }
+
+    if (!bindtextdomain(PACKAGE, LOCALEDIR)) {
+        perror("bindtextdomain");
+        exit(EXIT_FAILURE);
+    }
+
+    if (!textdomain(PACKAGE)) {
+        perror("textdomain");
+        exit(EXIT_FAILURE);
+    }
+
+    if (virThreadInitialize() < 0 ||
         virErrorInitialize() < 0) {
         fprintf(stderr, _("%s: initialization failed\n"), argv[0]);
         exit(EXIT_FAILURE);
diff --git a/src/locking/sanlock_helper.c b/src/locking/sanlock_helper.c
index d8d294f..6b17fce 100644
--- a/src/locking/sanlock_helper.c
+++ b/src/locking/sanlock_helper.c
@@ -70,10 +70,18 @@  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 (!setlocale(LC_ALL, "")) {
+        perror("setlocale");
+        /* failure to setup locale is not fatal */
+    }
+
+    if (!bindtextdomain(PACKAGE, LOCALEDIR)) {
+        perror("bindtextdomain");
+        exit(EXIT_FAILURE);
+    }
+
+    if (!textdomain(PACKAGE)) {
+        perror("textdomain");
         exit(EXIT_FAILURE);
     }
 
diff --git a/src/logging/log_daemon.c b/src/logging/log_daemon.c
index f674cbd..8a0de22 100644
--- a/src/logging/log_daemon.c
+++ b/src/logging/log_daemon.c
@@ -936,10 +936,22 @@  int main(int argc, char **argv) {
 
     privileged = geteuid() == 0;
 
-    if (setlocale(LC_ALL, "") == NULL ||
-        bindtextdomain(PACKAGE, LOCALEDIR) == NULL ||
-        textdomain(PACKAGE) == NULL ||
-        virThreadInitialize() < 0 ||
+    if (!setlocale(LC_ALL, "")) {
+        perror("setlocale");
+        /* failure to setup locale is not fatal */
+    }
+
+    if (!bindtextdomain(PACKAGE, LOCALEDIR)) {
+        perror("bindtextdomain");
+        exit(EXIT_FAILURE);
+    }
+
+    if (!textdomain(PACKAGE)) {
+        perror("textdomain");
+        exit(EXIT_FAILURE);
+    }
+
+    if (virThreadInitialize() < 0 ||
         virErrorInitialize() < 0) {
         fprintf(stderr, _("%s: initialization failed\n"), argv[0]);
         exit(EXIT_FAILURE);
diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c
index 8b5ec4c..612c0d7 100644
--- a/src/lxc/lxc_controller.c
+++ b/src/lxc/lxc_controller.c
@@ -2505,10 +2505,22 @@  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 ||
-        virThreadInitialize() < 0 ||
+    if (!setlocale(LC_ALL, "")) {
+        perror("setlocale");
+        /* failure to setup locale is not fatal */
+    }
+
+    if (!bindtextdomain(PACKAGE, LOCALEDIR)) {
+        perror("bindtextdomain");
+        exit(EXIT_FAILURE);
+    }
+
+    if (!textdomain(PACKAGE)) {
+        perror("textdomain");
+        exit(EXIT_FAILURE);
+    }
+
+    if (virThreadInitialize() < 0 ||
         virErrorInitialize() < 0) {
         fprintf(stderr, _("%s: initialization failed\n"), argv[0]);
         exit(EXIT_FAILURE);
diff --git a/src/network/leaseshelper.c b/src/network/leaseshelper.c
index 097cd11..e753e75 100644
--- a/src/network/leaseshelper.c
+++ b/src/network/leaseshelper.c
@@ -115,10 +115,18 @@  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);
+    if (!setlocale(LC_ALL, "")) {
+        perror("setlocale");
+        /* failure to setup locale is not fatal */
+    }
+
+    if (!bindtextdomain(PACKAGE, LOCALEDIR)) {
+        perror("bindtextdomain");
+        exit(EXIT_FAILURE);
+    }
+
+    if (!textdomain(PACKAGE)) {
+        perror("textdomain");
         exit(EXIT_FAILURE);
     }
 
diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
index 50d2a08..f47dc63 100644
--- a/src/security/virt-aa-helper.c
+++ b/src/security/virt-aa-helper.c
@@ -1298,10 +1298,18 @@  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]);
+    if (!setlocale(LC_ALL, "")) {
+        perror("setlocale");
+        /* failure to setup locale is not fatal */
+    }
+
+    if (!bindtextdomain(PACKAGE, LOCALEDIR)) {
+        perror("bindtextdomain");
+        exit(EXIT_FAILURE);
+    }
+
+    if (!textdomain(PACKAGE)) {
+        perror("textdomain");
         exit(EXIT_FAILURE);
     }
 
diff --git a/src/storage/parthelper.c b/src/storage/parthelper.c
index d1df068..c0f1f5a 100644
--- a/src/storage/parthelper.c
+++ b/src/storage/parthelper.c
@@ -72,10 +72,18 @@  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 (!setlocale(LC_ALL, "")) {
+        perror("setlocale");
+        /* failure to setup locale is not fatal */
+    }
+
+    if (!bindtextdomain(PACKAGE, LOCALEDIR)) {
+        perror("bindtextdomain");
+        exit(EXIT_FAILURE);
+    }
+
+    if (!textdomain(PACKAGE)) {
+        perror("textdomain");
         exit(EXIT_FAILURE);
     }
 
diff --git a/src/util/iohelper.c b/src/util/iohelper.c
index 8a3c377..0200bb1 100644
--- a/src/util/iohelper.c
+++ b/src/util/iohelper.c
@@ -230,10 +230,18 @@  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);
+    if (!setlocale(LC_ALL, "")) {
+        perror("setlocale");
+        /* failure to setup locale is not fatal */
+    }
+
+    if (!bindtextdomain(PACKAGE, LOCALEDIR)) {
+        perror("bindtextdomain");
+        exit(EXIT_FAILURE);
+    }
+
+    if (!textdomain(PACKAGE)) {
+        perror("textdomain");
         exit(EXIT_FAILURE);
     }