[libvirt] [PATCH] Be consistent with setlocale error handling
Cole Robinson
crobinso at redhat.com
Tue Apr 12 14:11:43 UTC 2016
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
More information about the libvir-list
mailing list