[pulseaudio-discuss] [PATCH 07/30] card: Use pa_card_free() in pa_card_new() on failure

[Tanu Kaskinen]

This makes it easier to ensure that resources are not leaked when new
failure conditions are added to pa_card_new() (this patch adds one
such condition: if the card has no profiles, pa_card_new() will fail).
---
 src/pulsecore/card.c | 68 ++++++++++++++++++++++++++++++----------------------
 1 file changed, 39 insertions(+), 29 deletions(-)

diff --git a/src/pulsecore/card.c b/src/pulsecore/card.c
index 3de2787..b780c5b 100644
--- a/src/pulsecore/card.c
+++ b/src/pulsecore/card.c
@@ -191,8 +191,7 @@ void pa_card_new_data_done(pa_card_new_data *data) {
 }
 
 pa_card *pa_card_new(pa_core *core, pa_card_new_data *data) {
-    pa_card *c;
-    const char *name;
+    pa_card *c = NULL;
     void *state;
     pa_card_profile *profile;
     pa_device_port *port;
@@ -203,23 +202,21 @@ pa_card *pa_card_new(pa_core *core, pa_card_new_data *data) {
     pa_assert(data->profiles);
     pa_assert(!pa_hashmap_isempty(data->profiles));
 
-    c = pa_xnew(pa_card, 1);
+    c = pa_xnew0(pa_card, 1);
+    c->core = core;
+    pa_assert_se(pa_idxset_put(core->cards, c, &c->index) >= 0);
 
-    if (!(name = pa_namereg_register(core, data->name, PA_NAMEREG_CARD, c, data->namereg_fail))) {
-        pa_xfree(c);
-        return NULL;
+    c->name = pa_xstrdup(pa_namereg_register(core, data->name, PA_NAMEREG_CARD, c, data->namereg_fail));
+    if (!(c->name)) {
+        pa_log("Failed to register card name '%s'.", data->name);
+        goto fail;
     }
 
-    pa_card_new_data_set_name(data, name);
+    pa_card_new_data_set_name(data, c->name);
 
-    if (pa_hook_fire(&core->hooks[PA_CORE_HOOK_CARD_NEW], data) < 0) {
-        pa_xfree(c);
-        pa_namereg_unregister(core, name);
-        return NULL;
-    }
+    if (pa_hook_fire(&core->hooks[PA_CORE_HOOK_CARD_NEW], data) < 0)
+        goto fail;
 
-    c->core = core;
-    c->name = pa_xstrdup(data->name);
     c->proplist = pa_proplist_copy(data->proplist);
     c->driver = pa_xstrdup(pa_path_get_filename(data->driver));
     c->module = data->module;
@@ -242,9 +239,7 @@ pa_card *pa_card_new(pa_core *core, pa_card_new_data *data) {
     PA_HASHMAP_FOREACH(port, c->ports, state)
         port->card = c;
 
-    c->active_profile = NULL;
     c->recreate_devices_on_profile_switch = data->recreate_devices_on_profile_switch;
-    c->save_profile = false;
 
     if (data->active_profile)
         if ((c->active_profile = pa_hashmap_get(c->profiles, data->active_profile)))
@@ -256,20 +251,26 @@ pa_card *pa_card_new(pa_core *core, pa_card_new_data *data) {
                 c->active_profile = profile;
     }
 
-    c->userdata = NULL;
-    c->set_profile = NULL;
+    if (!c->active_profile) {
+        pa_log("No profiles in card %s.", c->name);
+        goto fail;
+    }
 
     pa_device_init_description(c->proplist);
     pa_device_init_icon(c->proplist, true);
     pa_device_init_intended_roles(c->proplist);
 
-    pa_assert_se(pa_idxset_put(core->cards, c, &c->index) >= 0);
-
     pa_log_info("Created %u \"%s\"", c->index, c->name);
     pa_subscription_post(core, PA_SUBSCRIPTION_EVENT_CARD|PA_SUBSCRIPTION_EVENT_NEW, c->index);
 
     pa_hook_fire(&core->hooks[PA_CORE_HOOK_CARD_PUT], c);
     return c;
+
+fail:
+    if (c)
+        pa_card_free(c);
+
+    return NULL;
 }
 
 void pa_card_free(pa_card *c) {
@@ -282,28 +283,37 @@ void pa_card_free(pa_card *c) {
 
     pa_hook_fire(&core->hooks[PA_CORE_HOOK_CARD_UNLINK], c);
 
-    pa_namereg_unregister(core, c->name);
+    if (c->name)
+        pa_namereg_unregister(core, c->name);
 
-    pa_idxset_remove_by_data(c->core->cards, c, NULL);
+    pa_idxset_remove_by_data(core->cards, c, NULL);
 
-    pa_log_info("Freed %u \"%s\"", c->index, c->name);
+    pa_log_info("Freed %u \"%s\"", c->index, pa_strempty(c->name));
 
     pa_subscription_post(c->core, PA_SUBSCRIPTION_EVENT_CARD|PA_SUBSCRIPTION_EVENT_REMOVE, c->index);
 
-    pa_assert(pa_idxset_isempty(c->sinks));
-    pa_idxset_free(c->sinks, NULL);
-    pa_assert(pa_idxset_isempty(c->sources));
-    pa_idxset_free(c->sources, NULL);
+    if (c->sinks) {
+        pa_assert(pa_idxset_isempty(c->sinks));
+        pa_idxset_free(c->sinks, NULL);
+    }
+
+    if (c->sources) {
+        pa_assert(pa_idxset_isempty(c->sources));
+        pa_idxset_free(c->sources, NULL);
+    }
 
     if (c->device_prototypes)
         pa_hashmap_free(c->device_prototypes);
 
-    pa_hashmap_free(c->ports);
+    if (c->ports)
+        pa_hashmap_free(c->ports);
 
     if (c->profiles)
         pa_hashmap_free(c->profiles);
 
-    pa_proplist_free(c->proplist);
+    if (c->proplist)
+        pa_proplist_free(c->proplist);
+
     pa_xfree(c->driver);
     pa_xfree(c->name);
     pa_xfree(c);
-- 
1.8.3.1