[pulseaudio-discuss] [PATCH] Don't let user-set PULSE_RUNTIME_PATH values affect behaviour
David Henningsson
david.henningsson at canonical.com
Mon Sep 29 04:50:03 PDT 2014
On 2014-09-28 11:23, Tanu Kaskinen wrote:
> 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.
A quick googling of PULSE_RUNTIME_PATH seems to indicate usage of this
environment variable in at least chromium and enlightenment, and also
recommended in several blog posts and mailing lists, including this one.
It is likely used in several home-made scripts.
I'm hesitant to remove it for that reason.
The parts that just refactors "unsetenv" into "pa_unset_env" are acked
(and probably should have been in a separate patch anyway).
> ---
> 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.
>
--
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic
More information about the pulseaudio-discuss
mailing list