[pulseaudio-discuss] [PATCH] Don't let user-set PULSE_RUNTIME_PATH values affect behaviour

[Tanu Kaskinen]

The logic for choosing the runtime directory is complicated enough
also without adding PULSE_RUNTIME_PATH into the mix. XDG_RUNTIME_DIR
is sufficient for users to control the runtime directory.
PULSE_RUNTIME_PATH has not been documented, so this change doesn't
constitute an interface break.
---
 src/daemon/main.c           | 15 ++++++++++++---
 src/pulse/context.c         | 10 ++++++++++
 src/pulsecore/core-util.c   | 18 +++++++++++++-----
 src/pulsecore/core-util.h   |  1 +
 src/pulsecore/start-child.c |  2 +-
 src/tests/test-daemon.sh    |  4 ++--
 6 files changed, 39 insertions(+), 11 deletions(-)

diff --git a/src/daemon/main.c b/src/daemon/main.c
index 4fd245a..a81cbe0 100644
--- a/src/daemon/main.c
+++ b/src/daemon/main.c
@@ -232,9 +232,8 @@ static int change_user(void) {
     pa_set_env("LOGNAME", PA_SYSTEM_USER);
     pa_set_env("HOME", PA_SYSTEM_RUNTIME_PATH);
 
-    /* Relevant for pa_runtime_path() */
-    if (!getenv("PULSE_RUNTIME_PATH"))
-        pa_set_env("PULSE_RUNTIME_PATH", PA_SYSTEM_RUNTIME_PATH);
+    /* Relevant for pa_runtime_path(). */
+    pa_set_env("PULSE_RUNTIME_PATH", PA_SYSTEM_RUNTIME_PATH);
 
     if (!getenv("PULSE_CONFIG_PATH"))
         pa_set_env("PULSE_CONFIG_PATH", PA_SYSTEM_CONFIG_PATH);
@@ -452,6 +451,16 @@ int main(int argc, char *argv[]) {
     setlocale(LC_ALL, "");
     pa_init_i18n();
 
+    /* pa_runtime_dir() uses this environment variable to get the runtime
+     * directory, but the variable is supposed to be set only in the system
+     * mode by change_user(). User-set values should be ignored, which is why
+     * we unset the variable here. Users can use XDG_RUNTIME_DIR to control the
+     * runtime directory location.
+     *
+     * TODO: There's probably some better way to get the runtime dir in the
+     * system mode than relying on an internally-set environment variable... */
+    pa_unset_env("PULSE_RUNTIME_PATH");
+
     conf = pa_daemon_conf_new();
 
     if (pa_daemon_conf_load(conf, NULL) < 0)
diff --git a/src/pulse/context.c b/src/pulse/context.c
index 8897de0..6a0d66a 100644
--- a/src/pulse/context.c
+++ b/src/pulse/context.c
@@ -135,6 +135,16 @@ pa_context *pa_context_new_with_proplist(pa_mainloop_api *mainloop, const char *
 
     pa_init_i18n();
 
+    /* pa_runtime_dir() uses this environment variable to get the runtime
+     * directory, but the variable is supposed to be set only in the system
+     * mode. User-set values should be ignored, which is why we unset the
+     * variable here. Users can use XDG_RUNTIME_DIR to control the runtime
+     * directory location.
+     *
+     * TODO: There's probably some better way to get the runtime dir in the
+     * system mode than relying on an internally-set environment variable... */
+    pa_unset_env("PULSE_RUNTIME_PATH");
+
     c = pa_xnew0(pa_context, 1);
     PA_REFCNT_INIT(c);
 
diff --git a/src/pulsecore/core-util.c b/src/pulsecore/core-util.c
index 6b7cd35..e9843ef 100644
--- a/src/pulsecore/core-util.c
+++ b/src/pulsecore/core-util.c
@@ -2859,6 +2859,18 @@ void pa_set_env(const char *key, const char *value) {
 #endif
 }
 
+void pa_unset_env(const char *key) {
+    pa_assert(key);
+
+    /* This is not thread-safe */
+
+#ifdef OS_IS_WIN32
+    SetEnvironmentVariable(key, NULL);
+#else
+    unsetenv(key);
+#endif
+}
+
 void pa_set_env_and_record(const char *key, const char *value) {
     pa_assert(key);
     pa_assert(value);
@@ -2881,11 +2893,7 @@ void pa_unset_env_recorded(void) {
         if (!s)
             break;
 
-#ifdef OS_IS_WIN32
-        SetEnvironmentVariable(s, NULL);
-#else
-        unsetenv(s);
-#endif
+        pa_unset_env(s);
         pa_xfree(s);
     }
 }
diff --git a/src/pulsecore/core-util.h b/src/pulsecore/core-util.h
index d717299..8db56c5 100644
--- a/src/pulsecore/core-util.h
+++ b/src/pulsecore/core-util.h
@@ -210,6 +210,7 @@ int pa_reset_sigs(int except, ...);
 int pa_reset_sigsv(const int except[]);
 
 void pa_set_env(const char *key, const char *value);
+void pa_unset_env(const char *key);
 void pa_set_env_and_record(const char *key, const char *value);
 void pa_unset_env_recorded(void);
 
diff --git a/src/pulsecore/start-child.c b/src/pulsecore/start-child.c
index 7f55d4e..8318b9b 100644
--- a/src/pulsecore/start-child.c
+++ b/src/pulsecore/start-child.c
@@ -92,7 +92,7 @@ int pa_start_child_for_read(const char *name, const char *argv1, pid_t *pid) {
 
         /* Make sure our children are not influenced by the
          * LD_BIND_NOW we set for ourselves. */
-        unsetenv("LD_BIND_NOW");
+        pa_unset_env("LD_BIND_NOW");
 
 #ifdef PR_SET_PDEATHSIG
         /* On Linux we can use PR_SET_PDEATHSIG to have the helper
diff --git a/src/tests/test-daemon.sh b/src/tests/test-daemon.sh
index c1907a1..f042094 100755
--- a/src/tests/test-daemon.sh
+++ b/src/tests/test-daemon.sh
@@ -31,8 +31,8 @@ fi
 
 echo "Started bus pid $DBUS_SESSION_BUS_PID at $DBUS_SESSION_BUS_ADDRESS" >&2
 
-TEMP_PULSE_DIR=`mktemp -d`
-export PULSE_RUNTIME_PATH=${TEMP_PULSE_DIR}
+TEMP_RUNTIME_DIR=`mktemp -d`
+export XDG_RUNTIME_DIR=${TEMP_RUNTIME_DIR}
 
 # this script would be called inside src/ directory, so we need to use the correct path.
 # notice that for tests, we don't load ALSA related modules.
-- 
1.9.3