cli-command: don't exit on "module already loaded" errors

Some modules may only be loaded once, and trying to load them
twice from default.pa makes PulseAudio startup fail. While that could
be considered a user error, it's nicer to not be so strict. It's not
necessarily easy to figure what went wrong, if for example the user
plays with RAOP and adds module-raop-discover to default.pa, which first
works fine, but suddenly stops working when the user at some point
enables RAOP support in paprefs. Enabling RAOP in paprefs makes
module-gconf load the module too, so the module gets loaded twice.

This patch adds a way to differentiate module load errors, and
make cli-command ignore the error when the module is already
loaded.
This commit is contained in:
Colin Leroy 2017-08-26 11:21:15 +02:00 committed by Tanu Kaskinen
parent 1a66715320
commit f0dfddead3
23 changed files with 62 additions and 36 deletions

View file

@ -111,19 +111,21 @@ int deny_severity = LOG_WARNING;
int __padsp_disabled__ = 7;
#endif
static void signal_callback(pa_mainloop_api*m, pa_signal_event *e, int sig, void *userdata) {
static void signal_callback(pa_mainloop_api* m, pa_signal_event *e, int sig, void *userdata) {
pa_module *module = NULL;
pa_log_info("Got signal %s.", pa_sig2str(sig));
switch (sig) {
#ifdef SIGUSR1
case SIGUSR1:
pa_module_load(userdata, "module-cli", NULL);
pa_module_load(&module, userdata, "module-cli", NULL);
break;
#endif
#ifdef SIGUSR2
case SIGUSR2:
pa_module_load(userdata, "module-cli-protocol-unix", NULL);
pa_module_load(&module, userdata, "module-cli-protocol-unix", NULL);
break;
#endif

View file

@ -52,13 +52,13 @@ int pa__init(pa_module* m) {
u->bluez4_module_idx = PA_INVALID_INDEX;
if (pa_module_exists("module-bluez5-discover")) {
mm = pa_module_load(m->core, "module-bluez5-discover", m->argument);
pa_module_load(&mm, m->core, "module-bluez5-discover", m->argument);
if (mm)
u->bluez5_module_idx = mm->index;
}
if (pa_module_exists("module-bluez4-discover")) {
mm = pa_module_load(m->core, "module-bluez4-discover", NULL);
pa_module_load(&mm, m->core, "module-bluez4-discover", NULL);
if (mm)
u->bluez4_module_idx = mm->index;
}

View file

@ -71,6 +71,7 @@ static pa_hook_result_t source_put_hook_callback(pa_core *c, pa_source *source,
const char *s;
const char *role;
char *args;
pa_module *m = NULL;
pa_assert(c);
pa_assert(source);
@ -100,7 +101,7 @@ static pa_hook_result_t source_put_hook_callback(pa_core *c, pa_source *source,
/* Load module-loopback */
args = pa_sprintf_malloc("source=\"%s\" source_dont_move=\"true\" sink_input_properties=\"media.role=%s\"", source->name,
role);
(void) pa_module_load(c, "module-loopback", args);
(void) pa_module_load(&m, c, "module-loopback", args);
pa_xfree(args);
return PA_HOOK_OK;
@ -112,6 +113,7 @@ static pa_hook_result_t sink_put_hook_callback(pa_core *c, pa_sink *sink, void *
const char *s;
const char *role;
char *args;
pa_module *m = NULL;
pa_assert(c);
pa_assert(sink);
@ -139,7 +141,7 @@ static pa_hook_result_t sink_put_hook_callback(pa_core *c, pa_sink *sink, void *
/* Load module-loopback */
args = pa_sprintf_malloc("sink=\"%s\" sink_dont_move=\"true\" source_output_properties=\"media.role=%s\"", sink->name,
role);
(void) pa_module_load(c, "module-loopback", args);
(void) pa_module_load(&m, c, "module-loopback", args);
pa_xfree(args);
return PA_HOOK_OK;

View file

@ -93,7 +93,7 @@ static pa_hook_result_t load_module_for_device(pa_bluez4_discovery *y, const pa_
}
pa_log_debug("Loading module-bluez4-device %s", args);
m = pa_module_load(u->module->core, "module-bluez4-device", args);
pa_module_load(&m, u->module->core, "module-bluez4-device", args);
pa_xfree(args);
if (m) {

View file

@ -76,7 +76,7 @@ static pa_hook_result_t device_connection_changed_cb(pa_bluetooth_discovery *y,
char *args = pa_sprintf_malloc("path=%s autodetect_mtu=%i", d->path, (int)u->autodetect_mtu);
pa_log_debug("Loading module-bluez5-device %s", args);
m = pa_module_load(u->module->core, "module-bluez5-device", args);
pa_module_load(&m, u->module->core, "module-bluez5-device", args);
pa_xfree(args);
if (m)

View file

@ -1506,7 +1506,7 @@ static void handle_load_module(DBusConnection *conn, DBusMessage *msg, void *use
arg_string = pa_strbuf_to_string(arg_buffer);
if (!(module = pa_module_load(c->core, name, arg_string))) {
if (pa_module_load(&module, c->core, name, arg_string) < 0) {
pa_dbus_send_error(conn, msg, DBUS_ERROR_FAILED, "Failed to load module.");
goto finish;
}

View file

@ -190,7 +190,7 @@ static void load_module(
m->items[i].args = pa_xstrdup(args);
m->items[i].index = PA_INVALID_INDEX;
if (!(mod = pa_module_load(u->core, name, args))) {
if (pa_module_load(&mod, u->core, name, args) < 0) {
pa_log("pa_module_load() failed");
return;
}

View file

@ -111,7 +111,7 @@ static void ensure_ports_started(struct userdata* u) {
} else {
args = pa_sprintf_malloc("connect=%s", pa_yes_no(u->autoconnect_ports));
}
m = pa_module_load(u->core, modnames[i], args);
pa_module_load(&m, u->core, modnames[i], args);
pa_xfree(args);
if (m) {

View file

@ -92,7 +92,7 @@ static int ca_device_added(struct pa_module *m, AudioObjectID id) {
args = pa_sprintf_malloc("object_id=%d", (int) id);
pa_log_debug("Loading %s with arguments '%s'", DEVICE_MODULE_NAME, args);
mod = pa_module_load(m->core, DEVICE_MODULE_NAME, args);
pa_module_load(&mod, m->core, DEVICE_MODULE_NAME, args);
pa_xfree(args);
if (!mod) {

View file

@ -71,7 +71,7 @@ static pa_sink *ensure_null_sink_for_sink(struct userdata *u, pa_sink *s, pa_cor
t = pa_sprintf_malloc("sink_name=allow_passthrough_null_%s sink_properties='device.description=\"%s\"'",
name ? name : "", _("Dummy Output"));
m = pa_module_load(c, "module-null-sink", t);
pa_module_load(&m, c, "module-null-sink", t);
pa_xfree(t);
if (m == NULL)

View file

@ -80,7 +80,7 @@ static void load_null_sink_if_needed(pa_core *c, pa_sink *sink, struct userdata*
t = pa_sprintf_malloc("sink_name=%s sink_properties='device.description=\"%s\"'", u->sink_name,
_("Dummy Output"));
m = pa_module_load(c, "module-null-sink", t);
pa_module_load(&m, c, "module-null-sink", t);
u->null_module = m ? m->index : PA_INVALID_INDEX;
pa_xfree(t);

View file

@ -48,7 +48,7 @@ int pa__init(pa_module*m) {
pa_log_warn("We will now load module-combine-sink. Please make sure to remove module-combine from your configuration.");
module = pa_module_load(m->core, "module-combine-sink", m->argument);
pa_module_load(&module, m->core, "module-combine-sink", m->argument);
u->module_index = module ? module->index : PA_INVALID_INDEX;
return module ? 0 : -1;

View file

@ -74,6 +74,7 @@ static int detect_alsa(pa_core *c, int just_one) {
char line[64], args[64];
unsigned device, subdevice;
int is_sink;
pa_module *m = NULL;
if (!fgets(line, sizeof(line), f))
break;
@ -101,7 +102,7 @@ static int detect_alsa(pa_core *c, int just_one) {
continue;
pa_snprintf(args, sizeof(args), "device_id=%u", device);
if (!pa_module_load(c, is_sink ? "module-alsa-sink" : "module-alsa-source", args))
if (pa_module_load(&m, c, is_sink ? "module-alsa-sink" : "module-alsa-source", args) < 0)
continue;
n++;
@ -136,6 +137,7 @@ static int detect_oss(pa_core *c, int just_one) {
while (!feof(f)) {
char line[256], args[64];
unsigned device;
pa_module *m = NULL;
if (!fgets(line, sizeof(line), f))
break;
@ -156,14 +158,14 @@ static int detect_oss(pa_core *c, int just_one) {
else
pa_snprintf(args, sizeof(args), "device=/dev/dsp%u", device);
if (!pa_module_load(c, "module-oss", args))
if (pa_module_load(&m, c, "module-oss", args) < 0)
continue;
} else if (sscanf(line, "pcm%u: ", &device) == 1) {
/* FreeBSD support, the devices are named /dev/dsp0.0, dsp0.1 and so on */
pa_snprintf(args, sizeof(args), "device=/dev/dsp%u.0", device);
if (!pa_module_load(c, "module-oss", args))
if (pa_module_load(&m, c, "module-oss", args) < 0)
continue;
}
@ -183,6 +185,7 @@ static int detect_solaris(pa_core *c, int just_one) {
struct stat s;
const char *dev;
char args[64];
pa_module *m = NULL;
dev = getenv("AUDIODEV");
if (!dev)
@ -199,7 +202,7 @@ static int detect_solaris(pa_core *c, int just_one) {
pa_snprintf(args, sizeof(args), "device=%s", dev);
if (!pa_module_load(c, "module-solaris", args))
if (pa_module_load(&m, c, "module-solaris", args) < 0)
return 0;
return 1;
@ -208,11 +211,12 @@ static int detect_solaris(pa_core *c, int just_one) {
#ifdef OS_IS_WIN32
static int detect_waveout(pa_core *c, int just_one) {
pa_module *m = NULL;
/*
* FIXME: No point in enumerating devices until the plugin supports
* selecting anything but the first.
*/
if (!pa_module_load(c, "module-waveout", ""))
if (pa_module_load(&m, c, "module-waveout", "") < 0)
return 0;
return 1;

View file

@ -574,7 +574,7 @@ static pa_hook_result_t process(struct userdata *u, pa_object *o, bool is_sink_i
pa_log_debug("Loading %s with arguments '%s'", module_name, args);
if ((m = pa_module_load(u->core, module_name, args))) {
if (pa_module_load(&m, u->core, module_name, args) >= 0) {
find_filters_for_module(u, m, want, parameters);
filter = pa_hashmap_get(u->filters, fltr);
done_something = true;

View file

@ -64,7 +64,7 @@ int pa__init(pa_module*m) {
pa_log_warn("We will now load module-udev-detect. Please make sure to remove module-hal-detect from your configuration.");
t = pa_sprintf_malloc("tsched=%s", pa_yes_no(tsched));
n = pa_module_load(m->core, "module-udev-detect", t);
pa_module_load(&n, m->core, "module-udev-detect", t);
pa_xfree(t);
if (n)

View file

@ -332,7 +332,7 @@ static void verify_access(struct userdata *u, struct device *d) {
if (pa_ratelimit_test(&d->ratelimit, PA_LOG_DEBUG)) {
pa_log_debug("Loading module-alsa-card with arguments '%s'", d->args);
m = pa_module_load(u->core, "module-alsa-card", d->args);
pa_module_load(&m, u->core, "module-alsa-card", d->args);
if (m) {
d->module = m->index;

View file

@ -65,7 +65,7 @@ int pa__init(pa_module*m) {
pa_log_warn("We will now load module-stream-restore. Please make sure to remove module-volume-restore from your configuration.");
t = pa_sprintf_malloc("restore_volume=%s restore_device=%s", pa_yes_no(restore_volume), pa_yes_no(restore_device));
n = pa_module_load(m->core, "module-stream-restore", t);
pa_module_load(&n, m->core, "module-stream-restore", t);
pa_xfree(t);
if (n)

View file

@ -242,7 +242,7 @@ static void resolver_cb(
pa_log_debug("Loading %s with arguments '%s'", module_name, args);
if ((m = pa_module_load(u->core, module_name, args))) {
if (pa_module_load(&m, u->core, module_name, args) >= 0) {
tnl->module_index = m->index;
pa_hashmap_put(u->tunnels, tnl, tnl);
tnl = NULL;

View file

@ -310,7 +310,7 @@ static void resolver_cb(
pa_log_debug("Loading module-raop-sink with arguments '%s'", args);
if ((m = pa_module_load(u->core, "module-raop-sink", args))) {
if (pa_module_load(&m, u->core, "module-raop-sink", args) >= 0) {
tnl->module_index = m->index;
pa_hashmap_put(u->tunnels, tnl, tnl);
tnl = NULL;

View file

@ -421,6 +421,8 @@ static int pa_cli_command_info(pa_core *c, pa_tokenizer *t, pa_strbuf *buf, bool
static int pa_cli_command_load(pa_core *c, pa_tokenizer *t, pa_strbuf *buf, bool *fail) {
const char *name;
pa_error_code_t err;
pa_module *m = NULL;
pa_core_assert_ref(c);
pa_assert(t);
@ -432,9 +434,13 @@ static int pa_cli_command_load(pa_core *c, pa_tokenizer *t, pa_strbuf *buf, bool
return -1;
}
if (!pa_module_load(c, name, pa_tokenizer_get(t, 2))) {
pa_strbuf_puts(buf, "Module load failed.\n");
return -1;
if ((err = pa_module_load(&m, c, name, pa_tokenizer_get(t, 2))) < 0) {
if (err == PA_ERR_EXIST) {
pa_strbuf_puts(buf, "Module already loaded; ignoring.\n");
} else {
pa_strbuf_puts(buf, "Module load failed.\n");
return -1;
}
}
return 0;

View file

@ -107,17 +107,21 @@ void pa_module_hook_connect(pa_module *m, pa_hook *hook, pa_hook_priority_t prio
pa_dynarray_append(m->hooks, pa_hook_connect(hook, prio, cb, data));
}
pa_module* pa_module_load(pa_core *c, const char *name, const char *argument) {
int pa_module_load(pa_module** module, pa_core *c, const char *name, const char *argument) {
pa_module *m = NULL;
bool (*load_once)(void);
const char* (*get_deprecated)(void);
pa_modinfo *mi;
int errcode;
pa_assert(module);
pa_assert(c);
pa_assert(name);
if (c->disallow_module_loading)
if (c->disallow_module_loading) {
errcode = -PA_ERR_ACCESS;
goto fail;
}
m = pa_xnew(pa_module, 1);
m->name = pa_xstrdup(name);
@ -135,6 +139,7 @@ pa_module* pa_module_load(pa_core *c, const char *name, const char *argument) {
* loader, which never finds anything, and therefore says "file not
* found". */
pa_log("Failed to open module \"%s\".", name);
errcode = -PA_ERR_IO;
goto fail;
}
@ -150,6 +155,7 @@ pa_module* pa_module_load(pa_core *c, const char *name, const char *argument) {
PA_IDXSET_FOREACH(i, c->modules, idx) {
if (pa_streq(name, i->name)) {
pa_log("Module \"%s\" should be loaded once at most. Refusing to load.", name);
errcode = -PA_ERR_EXIST;
goto fail;
}
}
@ -165,6 +171,7 @@ pa_module* pa_module_load(pa_core *c, const char *name, const char *argument) {
if (!(m->init = (int (*)(pa_module*_m)) pa_load_sym(m->dl, name, PA_SYMBOL_INIT))) {
pa_log("Failed to load module \"%s\": symbol \""PA_SYMBOL_INIT"\" not found.", name);
errcode = -PA_ERR_IO;
goto fail;
}
@ -179,6 +186,7 @@ pa_module* pa_module_load(pa_core *c, const char *name, const char *argument) {
if (m->init(m) < 0) {
pa_log_error("Failed to load module \"%s\" (argument: \"%s\"): initialization failed.", name, argument ? argument : "");
errcode = -PA_ERR_IO;
goto fail;
}
@ -202,7 +210,9 @@ pa_module* pa_module_load(pa_core *c, const char *name, const char *argument) {
pa_hook_fire(&m->core->hooks[PA_CORE_HOOK_MODULE_NEW], m);
return m;
*module = m;
return 0;
fail:
@ -225,7 +235,9 @@ fail:
pa_xfree(m);
}
return NULL;
*module = NULL;
return errcode;
}
static void postponed_dlclose(pa_mainloop_api *api, void *userdata) {

View file

@ -52,7 +52,7 @@ struct pa_module {
bool pa_module_exists(const char *name);
pa_module* pa_module_load(pa_core *c, const char *name, const char *argument);
int pa_module_load(pa_module** m, pa_core *c, const char *name, const char *argument);
void pa_module_unload(pa_module *m, bool force);
void pa_module_unload_by_index(pa_core *c, uint32_t idx, bool force);

View file

@ -4468,7 +4468,7 @@ static void command_load_module(pa_pdispatch *pd, uint32_t command, uint32_t tag
CHECK_VALIDITY(c->pstream, name && *name && pa_utf8_valid(name) && !strchr(name, '/'), tag, PA_ERR_INVALID);
CHECK_VALIDITY(c->pstream, !argument || pa_utf8_valid(argument), tag, PA_ERR_INVALID);
if (!(m = pa_module_load(c->protocol->core, name, argument))) {
if (pa_module_load(&m, c->protocol->core, name, argument) < 0) {
pa_pstream_send_error(c->pstream, tag, PA_ERR_MODINITFAILED);
return;
}