From d9571c2f1cf612ce0d479e428f7b1ae24944d140 Mon Sep 17 00:00:00 2001

From: Vojtech Trefny <vtrefny@redhat.com>

Date: Wed, 10 Jun 2020 17:03:28 +0200

Subject: [PATCH] exec: Fix setting locale for util calls

This actually fixes two issue. The _utils_exec_and_report_progress

function didn't set the LC_ALL=C environment variable to make sure

we get output in English. And also we shouldn't use setenv in the

GSpawnChildSetupFunc, it's actually example of what not to do in

g_spawn_async documentation. This fix uses g_environ_setenv and

passes the new environment to the g_spawn call.

---

 src/utils/exec.c    | 26 +++++++++++++++++---------

 tests/utils_test.py |  7 +++++++

 2 files changed, 24 insertions(+), 9 deletions(-)

diff --git a/src/utils/exec.c b/src/utils/exec.c

index 9293930..37bd960 100644

--- a/src/utils/exec.c

+++ b/src/utils/exec.c

@@ -140,11 +140,6 @@ static void log_done (guint64 task_id, gint exit_code) {

     return;

 }

-static void set_c_locale(gpointer user_data __attribute__((unused))) {

-    if (setenv ("LC_ALL", "C", 1) != 0)

-        g_warning ("Failed to set LC_ALL=C for a child process!");

-}

-

 /**

  * bd_utils_exec_and_report_error:

  * @argv: (array zero-terminated=1): the argv array for the call

@@ -194,6 +189,8 @@ gboolean bd_utils_exec_and_report_status_error (const gchar **argv, const BDExtr

     const BDExtraArg **extra_p = NULL;

     gint exit_status = 0;

     guint i = 0;

+    gchar **old_env = NULL;

+    gchar **new_env = NULL;

     if (extra) {

         args_len = g_strv_length ((gchar **) argv);

@@ -219,16 +216,20 @@ gboolean bd_utils_exec_and_report_status_error (const gchar **argv, const BDExtr

         args[i] = NULL;

     }

+    old_env = g_get_environ ();

+    new_env = g_environ_setenv (old_env, "LC_ALL", "C", TRUE);

+

     task_id = log_running (args ? args : argv);

-    success = g_spawn_sync (NULL, args ? (gchar **) args : (gchar **) argv, NULL, G_SPAWN_SEARCH_PATH,

-                            (GSpawnChildSetupFunc) set_c_locale, NULL,

-                            &stdout_data, &stderr_data, &exit_status, error);

+    success = g_spawn_sync (NULL, args ? (gchar **) args : (gchar **) argv, new_env, G_SPAWN_SEARCH_PATH,

+                            NULL, NULL, &stdout_data, &stderr_data, &exit_status, error);

     if (!success) {

         /* error is already populated from the call */

+        g_strfreev (new_env);

         g_free (stdout_data);

         g_free (stderr_data);

         return FALSE;

     }

+    g_strfreev (new_env);

     /* g_spawn_sync set the status in the same way waitpid() does, we need

        to get the process exit code manually (this is similar to calling

@@ -297,6 +298,8 @@ static gboolean _utils_exec_and_report_progress (const gchar **argv, const BDExt

     gboolean err_done = FALSE;

     GString *stdout_data = g_string_new (NULL);

     GString *stderr_data = g_string_new (NULL);

+    gchar **old_env = NULL;

+    gchar **new_env = NULL;

     /* TODO: share this code between functions */

     if (extra) {

@@ -325,7 +328,10 @@ static gboolean _utils_exec_and_report_progress (const gchar **argv, const BDExt

     task_id = log_running (args ? args : argv);

-    ret = g_spawn_async_with_pipes (NULL, args ? (gchar**) args : (gchar**) argv, NULL,

+    old_env = g_get_environ ();

+    new_env = g_environ_setenv (old_env, "LC_ALL", "C", TRUE);

+

+    ret = g_spawn_async_with_pipes (NULL, args ? (gchar**) args : (gchar**) argv, new_env,

                                     G_SPAWN_DEFAULT|G_SPAWN_SEARCH_PATH|G_SPAWN_DO_NOT_REAP_CHILD,

                                     NULL, NULL, &pid, NULL, &out_fd, &err_fd, error);

@@ -333,9 +339,11 @@ static gboolean _utils_exec_and_report_progress (const gchar **argv, const BDExt

         /* error is already populated */

         g_string_free (stdout_data, TRUE);

         g_string_free (stderr_data, TRUE);

+        g_strfreev (new_env);

         g_free (args);

         return FALSE;

     }

+    g_strfreev (new_env);

     args_str = g_strjoinv (" ", args ? (gchar **) args : (gchar **) argv);

     msg = g_strdup_printf ("Started '%s'", args_str);

diff --git a/tests/utils_test.py b/tests/utils_test.py

index 4bec3db..2bec5ed 100644

--- a/tests/utils_test.py

+++ b/tests/utils_test.py

@@ -170,6 +170,13 @@ class UtilsExecLoggingTest(UtilsTestCase):

             # exit code != 0

             self.assertTrue(BlockDev.utils_check_util_version("libblockdev-fake-util-fail", "1.1", "version", "Version:\\s(.*)"))

+    @tag_test(TestTags.NOSTORAGE, TestTags.CORE)

+    def test_exec_locale(self):

+        """Verify that setting locale for exec functions works as expected"""

+

+        succ, out = BlockDev.utils_exec_and_capture_output(["locale"])

+        self.assertTrue(succ)

+        self.assertIn("LC_ALL=C", out)

 class UtilsDevUtilsTestCase(UtilsTestCase):

     @tag_test(TestTags.NOSTORAGE, TestTags.CORE)

-- 

2.26.2