[pulseaudio-discuss] [PATCH 04/30] Allow pa_node_put() to fail

[Tanu Kaskinen]

The initial routing may fail, so pa_node_put() may fail too.

This patch adds dubious assertions in pa_sink_put() and
pa_source_put(). Those functions call pa_node_put(), which can fail,
but pa_sink_put() and pa_source_put() can't themselves fail. The
assertions may be OK, because at least at this point there's no
reason to fail the initial routing of sinks and sources, but I don't
think it's completely impossible that this might change at some point
in the future, and at that point it may be hard to notice those
assertions before they blow up in production. I don't think it would
be excessively difficult to change pa_sink_put() and pa_source_put()
so that they would allow failure, so I'm OK with doing that change if
others think that it's the best way forward.
---
 src/pulsecore/device-port.c   | 13 +++++++++++--
 src/pulsecore/node.c          |  4 +++-
 src/pulsecore/node.h          |  2 +-
 src/pulsecore/sink-input.c    |  7 ++++++-
 src/pulsecore/sink.c          |  7 +++++--
 src/pulsecore/source-output.c |  7 ++++++-
 src/pulsecore/source.c        |  7 +++++--
 7 files changed, 37 insertions(+), 10 deletions(-)

diff --git a/src/pulsecore/device-port.c b/src/pulsecore/device-port.c
index 6d340c7..76d52ae 100644
--- a/src/pulsecore/device-port.c
+++ b/src/pulsecore/device-port.c
@@ -121,6 +121,7 @@ static void device_port_free(pa_object *o) {
 
 pa_device_port *pa_device_port_new(pa_core *c, pa_device_port_new_data *data, size_t extra) {
     pa_device_port *p;
+    int r;
 
     pa_assert(data);
     pa_assert(data->name);
@@ -153,7 +154,11 @@ pa_device_port *pa_device_port_new(pa_core *c, pa_device_port_new_data *data, si
     }
 
     p->node->owner = p;
-    pa_node_put(p->node, NULL, 0);
+
+    r = pa_node_put(p->node, NULL, 0);
+
+    if (r < 0)
+        goto fail;
 
     if (p->direction == PA_DIRECTION_OUTPUT) {
         char *description;
@@ -177,7 +182,11 @@ pa_device_port *pa_device_port_new(pa_core *c, pa_device_port_new_data *data, si
         }
 
         p->monitor_node->owner = p;
-        pa_node_put(p->monitor_node, NULL, 0);
+
+        r = pa_node_put(p->monitor_node, NULL, 0);
+
+        if (r < 0)
+            goto fail;
     }
 
     return p;
diff --git a/src/pulsecore/node.c b/src/pulsecore/node.c
index 941e376..790c054 100644
--- a/src/pulsecore/node.c
+++ b/src/pulsecore/node.c
@@ -152,7 +152,7 @@ void pa_node_free(pa_node *node) {
     pa_xfree(node);
 }
 
-void pa_node_put(pa_node *node, pa_node **requested_connections, unsigned n_requested_connections) {
+int pa_node_put(pa_node *node, pa_node **requested_connections, unsigned n_requested_connections) {
     pa_assert(node);
     pa_assert(node->state == PA_NODE_STATE_INIT);
     pa_assert(node->owner);
@@ -167,6 +167,8 @@ void pa_node_put(pa_node *node, pa_node **requested_connections, unsigned n_requ
     pa_hook_fire(&node->core->hooks[PA_CORE_HOOK_NODE_SET_INITIAL_ROUTING], node);
 
     pa_log_debug("Created node %s.", node->name);
+
+    return 0;
 }
 
 void pa_node_unlink(pa_node *node) {
diff --git a/src/pulsecore/node.h b/src/pulsecore/node.h
index 7a8502d..7366fed 100644
--- a/src/pulsecore/node.h
+++ b/src/pulsecore/node.h
@@ -88,7 +88,7 @@ void pa_node_new_data_done(pa_node_new_data *data);
 pa_node *pa_node_new(pa_core *core, pa_node_new_data *data);
 void pa_node_free(pa_node *node);
 
-void pa_node_put(pa_node *node, pa_node **requested_connections, unsigned n_requested_connections);
+int pa_node_put(pa_node *node, pa_node **requested_connections, unsigned n_requested_connections);
 void pa_node_unlink(pa_node *node);
 
 #endif
diff --git a/src/pulsecore/sink-input.c b/src/pulsecore/sink-input.c
index e88477b..c4ab115 100644
--- a/src/pulsecore/sink-input.c
+++ b/src/pulsecore/sink-input.c
@@ -390,7 +390,12 @@ int pa_sink_input_new(
     i->node->owner = i;
 
     /* This may update i->sink and i->format. */
-    pa_node_put(i->node, &sink_node, sink_node ? 1 : 0);
+    r = pa_node_put(i->node, &sink_node, sink_node ? 1 : 0);
+
+    if (r < 0) {
+        ret = r;
+        goto fail;
+    }
 
     if (!i->sink) {
         pa_sink *sink = pa_namereg_get(core, NULL, PA_NAMEREG_SINK);
diff --git a/src/pulsecore/sink.c b/src/pulsecore/sink.c
index 0cf8a88..c2aea32 100644
--- a/src/pulsecore/sink.c
+++ b/src/pulsecore/sink.c
@@ -682,8 +682,11 @@ void pa_sink_put(pa_sink* s) {
 
     pa_source_put(s->monitor_source);
 
-    if (s->node)
-        pa_node_put(s->node, NULL, 0);
+    if (s->node) {
+        int r;
+        r = pa_node_put(s->node, NULL, 0);
+        pa_assert(r >= 0);
+    }
 
     pa_subscription_post(s->core, PA_SUBSCRIPTION_EVENT_SINK | PA_SUBSCRIPTION_EVENT_NEW, s->index);
     pa_hook_fire(&s->core->hooks[PA_CORE_HOOK_SINK_PUT], s);
diff --git a/src/pulsecore/source-output.c b/src/pulsecore/source-output.c
index 8eaf591..92a4510 100644
--- a/src/pulsecore/source-output.c
+++ b/src/pulsecore/source-output.c
@@ -323,7 +323,12 @@ int pa_source_output_new(
     o->node->owner = o;
 
     /* This may update o->source and o->format. */
-    pa_node_put(o->node, &source_node, source_node ? 1 : 0);
+    r = pa_node_put(o->node, &source_node, source_node ? 1 : 0);
+
+    if (r < 0) {
+        ret = r;
+        goto fail;
+    }
 
     if (!o->source) {
         pa_source *source;
diff --git a/src/pulsecore/source.c b/src/pulsecore/source.c
index ab0018d..0ccdcb3 100644
--- a/src/pulsecore/source.c
+++ b/src/pulsecore/source.c
@@ -636,8 +636,11 @@ void pa_source_put(pa_source *s) {
     else
         pa_assert_se(source_set_state(s, PA_SOURCE_IDLE) == 0);
 
-    if (s->node)
-        pa_node_put(s->node, NULL, 0);
+    if (s->node) {
+        int r;
+        r = pa_node_put(s->node, NULL, 0);
+        pa_assert(r >= 0);
+    }
 
     pa_subscription_post(s->core, PA_SUBSCRIPTION_EVENT_SOURCE | PA_SUBSCRIPTION_EVENT_NEW, s->index);
     pa_hook_fire(&s->core->hooks[PA_CORE_HOOK_SOURCE_PUT], s);
-- 
1.8.3.1