[libvirt] [PATCH] Be consistent with setlocale error handling

[Cole Robinson]

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