improve error handling

Make sure we free properties we take ownership of.
Add some more return values to functions that can fail.

spa/include/spa/support/loop.h

@@ -222,11 +222,11 @@ struct spa_loop_utils_methods {
 	struct spa_source *(*add_idle) (void *object,
 					bool enabled,
 					spa_source_idle_func_t func, void *data);
-	void (*enable_idle) (void *object, struct spa_source *source, bool enabled);
+	int (*enable_idle) (void *object, struct spa_source *source, bool enabled);
 
 	struct spa_source *(*add_event) (void *object,
 					 spa_source_event_func_t func, void *data);
-	void (*signal_event) (void *object, struct spa_source *source);
+	int (*signal_event) (void *object, struct spa_source *source);
 
 	struct spa_source *(*add_timer) (void *object,
 					 spa_source_timer_func_t func, void *data);
@@ -276,9 +276,9 @@ struct spa_loop_utils_methods {
 #define spa_loop_utils_add_io(l,...)		spa_loop_utils_method_s(l,add_io,0,__VA_ARGS__)
 #define spa_loop_utils_update_io(l,...)		spa_loop_utils_method_r(l,update_io,0,__VA_ARGS__)
 #define spa_loop_utils_add_idle(l,...)		spa_loop_utils_method_s(l,add_idle,0,__VA_ARGS__)
-#define spa_loop_utils_enable_idle(l,...)	spa_loop_utils_method_v(l,enable_idle,0,__VA_ARGS__)
+#define spa_loop_utils_enable_idle(l,...)	spa_loop_utils_method_r(l,enable_idle,0,__VA_ARGS__)
 #define spa_loop_utils_add_event(l,...)		spa_loop_utils_method_s(l,add_event,0,__VA_ARGS__)
-#define spa_loop_utils_signal_event(l,...)	spa_loop_utils_method_v(l,signal_event,0,__VA_ARGS__)
+#define spa_loop_utils_signal_event(l,...)	spa_loop_utils_method_r(l,signal_event,0,__VA_ARGS__)
 #define spa_loop_utils_add_timer(l,...)		spa_loop_utils_method_s(l,add_timer,0,__VA_ARGS__)
 #define spa_loop_utils_update_timer(l,...)	spa_loop_utils_method_r(l,update_timer,0,__VA_ARGS__)
 #define spa_loop_utils_add_signal(l,...)	spa_loop_utils_method_s(l,add_signal,0,__VA_ARGS__)

spa/plugins/support/loop.c

@@ -56,7 +56,7 @@ struct invoke_item {
 	int res;
 };
 
-static void loop_signal_event(void *object, struct spa_source *source);
+static int loop_signal_event(void *object, struct spa_source *source);
 
 struct impl {
 	struct spa_handle handle;
@@ -361,10 +361,10 @@ static void source_idle_func(struct spa_source *source)
 }
 
 
-static void loop_enable_idle(void *object, struct spa_source *source, bool enabled)
+static int loop_enable_idle(void *object, struct spa_source *source, bool enabled)
 {
 	struct source_impl *impl = SPA_CONTAINER_OF(source, struct source_impl, source);
-	int res;
+	int res = 0;
 
 	if (enabled && !impl->enabled) {
 		if ((res = spa_system_eventfd_write(impl->impl->system, source->fd, 1)) < 0)
@@ -377,6 +377,7 @@ static void loop_enable_idle(void *object, struct spa_source *source, bool enabl
 					source, source->fd, spa_strerror(res));
 	}
 	impl->enabled = enabled;
+	return res;
 }
 static struct spa_source *loop_add_idle(void *object,
 					bool enabled, spa_source_idle_func_t func, void *data)
@@ -474,7 +475,7 @@ err_free:
 	goto out;
 }
 
-static void loop_signal_event(void *object, struct spa_source *source)
+static int loop_signal_event(void *object, struct spa_source *source)
 {
 	struct source_impl *impl = SPA_CONTAINER_OF(source, struct source_impl, source);
 	int res;
@@ -482,6 +483,7 @@ static void loop_signal_event(void *object, struct spa_source *source)
 	if ((res = spa_system_eventfd_write(impl->impl->system, source->fd, 1)) < 0)
 		spa_log_warn(impl->impl->log, NAME " %p: failed to write event fd %d: %s",
 				source, source->fd, spa_strerror(res));
+	return res;
 }
 
 static void source_timer_func(struct spa_source *source)

src/pipewire/client.c

@@ -341,13 +341,10 @@ int pw_client_register(struct pw_client *client,
 	struct pw_core *core = client->core;
 
 	if (client->registered)
-		return -EEXIST;
+		goto error_existed;
 
 	pw_log_debug("client %p: register parent %d", client, parent ? parent->id : SPA_ID_INVALID);
 
-	spa_list_append(&core->client_list, &client->link);
-	client->registered = true;
-
 	client->global = pw_global_new(core,
 				       PW_TYPE_INTERFACE_Client,
 				       PW_VERSION_CLIENT_PROXY,
@@ -357,11 +354,19 @@ int pw_client_register(struct pw_client *client,
 	if (client->global == NULL)
 		return -errno;
 
+	spa_list_append(&core->client_list, &client->link);
+	client->registered = true;
+
 	pw_global_add_listener(client->global, &client->global_listener, &global_events, client);
 	pw_global_register(client->global, owner, parent);
 	client->info.id = client->global->id;
 
 	return 0;
+
+error_existed:
+	if (properties)
+		pw_properties_free(properties);
+	return -EEXIST;
 }
 
 SPA_EXPORT

src/pipewire/device.c

@@ -352,6 +352,9 @@ int pw_device_register(struct pw_device *device,
 	struct node_data *nd;
 	const char *str;
 
+	if (device->registered)
+		goto error_existed;
+
 	if (properties == NULL)
 		properties = pw_properties_new(NULL, NULL);
 	if (properties == NULL)
@@ -361,9 +364,6 @@ int pw_device_register(struct pw_device *device,
 	if ((str = pw_properties_get(device->properties, PW_KEY_MEDIA_CLASS)) != NULL)
 		pw_properties_set(properties, PW_KEY_MEDIA_CLASS, str);
 
-	spa_list_append(&core->device_list, &device->link);
-	device->registered = true;
-
         device->global = pw_global_new(core,
 				       PW_TYPE_INTERFACE_Device, PW_VERSION_DEVICE_PROXY,
 				       properties,
@@ -372,6 +372,9 @@ int pw_device_register(struct pw_device *device,
 	if (device->global == NULL)
 		return -errno;
 
+	spa_list_append(&core->device_list, &device->link);
+	device->registered = true;
+
 	device->info.id = device->global->id;
 	pw_global_add_listener(device->global, &device->global_listener, &global_events, device);
 	pw_global_register(device->global, owner, parent);
@@ -381,6 +384,11 @@ int pw_device_register(struct pw_device *device,
 		pw_node_set_active(nd->node, true);
 	}
 	return 0;
+
+error_existed:
+	if (properties)
+		pw_properties_free(properties);
+	return -EEXIST;
 }
 
 static void node_destroy(void *data)

src/pipewire/factory.c

@@ -153,7 +153,7 @@ int pw_factory_register(struct pw_factory *factory,
 	struct pw_core *core = factory->core;
 
 	if (factory->registered)
-		return -EEXIST;
+		goto error_existed;
 
 	if (properties == NULL)
 		properties = pw_properties_new(NULL, NULL);
@@ -165,9 +165,6 @@ int pw_factory_register(struct pw_factory *factory,
 			spa_debug_type_find_name(pw_type_info(), factory->info.type));
 	pw_properties_setf(properties, PW_KEY_FACTORY_TYPE_VERSION, "%d", factory->info.version);
 
-	spa_list_append(&core->factory_list, &factory->link);
-	factory->registered = true;
-
         factory->global = pw_global_new(core,
 					PW_TYPE_INTERFACE_Factory,
 					PW_VERSION_FACTORY_PROXY,
@@ -177,11 +174,19 @@ int pw_factory_register(struct pw_factory *factory,
 	if (factory->global == NULL)
 		return -errno;
 
+	spa_list_append(&core->factory_list, &factory->link);
+	factory->registered = true;
+
 	pw_global_add_listener(factory->global, &factory->global_listener, &global_events, factory);
 	pw_global_register(factory->global, owner, parent);
 	factory->info.id = factory->global->id;
 
 	return 0;
+
+error_existed:
+	if (properties)
+		pw_properties_free(properties);
+	return -EEXIST;
 }
 
 SPA_EXPORT

src/pipewire/global.c

@@ -92,8 +92,10 @@ pw_global_new(struct pw_core *core,
 	int res;
 
 	impl = calloc(1, sizeof(struct impl));
-	if (impl == NULL)
+	if (impl == NULL) {
-		return NULL;
+		res = -errno;
+		goto error_cleanup;
+	}
 
 	this = &impl->this;
 
@@ -107,7 +109,7 @@ pw_global_new(struct pw_core *core,
 	if (this->id == SPA_ID_INVALID) {
 		res = -errno;
 		pw_log_error("global %p: can't allocate new id: %m", this);
-		goto error_clean;
+		goto error_free;
 	}
 
 	spa_list_init(&this->child_list);
@@ -120,8 +122,11 @@ pw_global_new(struct pw_core *core,
 
 	return this;
 
-error_clean:
+error_free:
 	free(impl);
+error_cleanup:
+	if (properties)
+		pw_properties_free(properties);
 	errno = -res;
 	return NULL;
 }
@@ -145,6 +150,9 @@ pw_global_register(struct pw_global *global,
 	struct pw_resource *registry;
 	struct pw_core *core = global->core;
 
+	if (impl->registered)
+		return -EEXIST;
+
 	global->owner = owner;
 	if (owner && parent == NULL)
 		parent = owner->global;

src/pipewire/link.c

@@ -1253,24 +1253,24 @@ struct pw_link *pw_link_new(struct pw_core *core,
 	int res;
 
 	if (output == input)
-		goto same_ports;
+		goto error_same_ports;
 
 	if (output->direction != PW_DIRECTION_OUTPUT ||
 	    input->direction != PW_DIRECTION_INPUT)
-		goto wrong_direction;
+		goto error_wrong_direction;
 
 	if (pw_link_find(output, input))
-		goto link_exists;
+		goto error_link_exists;
 
 	if (check_permission(core, output, input, properties) < 0)
-		goto link_not_allowed;
+		goto error_link_not_allowed;
 
 	input_node = input->node;
 	output_node = output->node;
 
 	impl = calloc(1, sizeof(struct impl) + user_data_size);
 	if (impl == NULL)
-		goto no_mem;
+		goto error_no_mem;
 
 	this = &impl->this;
 	this->feedback = pw_node_can_reach(input_node, output_node);
@@ -1322,7 +1322,7 @@ struct pw_link *pw_link_new(struct pw_core *core,
 	pw_port_init_mix(input, &this->rt.in_mix);
 
 	if ((res = select_io(this)) < 0)
-		goto no_io;
+		goto error_no_io;
 
 	if (this->feedback) {
 		impl->inode = output_node;
@@ -1351,28 +1351,35 @@ struct pw_link *pw_link_new(struct pw_core *core,
 
 	return this;
 
-      no_io:
+error_same_ports:
-	pw_log_error("can't set io %d (%s)", res, spa_strerror(res));
+	res = -EINVAL;
-	errno = -res;
-	return NULL;
-      same_ports:
 	pw_log_error("can't link the same ports");
-	errno = EINVAL;
+	goto error_exit;
-	return NULL;
+error_wrong_direction:
-      wrong_direction:
+	res = -EINVAL;
 	pw_log_error("ports have wrong direction");
-	errno = EINVAL;
+	goto error_exit;
-	return NULL;
+error_link_exists:
-      link_exists:
+	res = -EEXIST;
 	pw_log_error("link already exists");
-	errno = EEXIST;
+	goto error_exit;
-	return NULL;
+error_link_not_allowed:
-      link_not_allowed:
+	res = -EPERM;
 	pw_log_error("link not allowed");
-	errno = EPERM;
+	goto error_exit;
-	return NULL;
+error_no_mem:
-      no_mem:
+	res = -errno;
-	pw_log_error("no memory");
+	pw_log_error("alloc failed: %m");
+	goto error_exit;
+error_no_io:
+	pw_log_error("can't set io %d (%s)", res, spa_strerror(res));
+	goto error_free;
+error_free:
+	free(impl);
+error_exit:
+	if (properties)
+		pw_properties_free(properties);
+	errno = -res;
 	return NULL;
 }
 
@@ -1399,7 +1406,7 @@ int pw_link_register(struct pw_link *link,
 	struct pw_node *input_node, *output_node;
 
 	if (link->registered)
-		return -EEXIST;
+		goto error_existed;
 
 	if (properties == NULL)
 		properties = pw_properties_new(NULL, NULL);
@@ -1417,9 +1424,6 @@ int pw_link_register(struct pw_link *link,
 	pw_properties_setf(properties, PW_KEY_LINK_INPUT_PORT, "%d", link->info.input_port_id);
 	pw_properties_setf(properties, PW_KEY_LINK_OUTPUT_PORT, "%d", link->info.output_port_id);
 
-	spa_list_append(&core->link_list, &link->link);
-	link->registered = true;
-
 	link->global = pw_global_new(core,
 				     PW_TYPE_INTERFACE_Link,
 				     PW_VERSION_LINK_PROXY,
@@ -1429,9 +1433,11 @@ int pw_link_register(struct pw_link *link,
 	if (link->global == NULL)
 		return -errno;
 
-	pw_global_add_listener(link->global, &link->global_listener, &global_events, link);
+	spa_list_append(&core->link_list, &link->link);
+	link->registered = true;
 
 	link->info.id = link->global->id;
+	pw_global_add_listener(link->global, &link->global_listener, &global_events, link);
 	pw_global_register(link->global, owner, parent);
 
 	debug_link(link);
@@ -1442,6 +1448,11 @@ int pw_link_register(struct pw_link *link,
 		pw_link_prepare(link);
 
 	return 0;
+
+error_existed:
+	if (properties)
+		pw_properties_free(properties);
+	return -EEXIST;
 }
 
 SPA_EXPORT

src/pipewire/loop.c

@@ -62,8 +62,10 @@ struct pw_loop *pw_loop_new(struct pw_properties *properties)
 	n_support = pw_get_support(support, 32);
 
 	impl = calloc(1, sizeof(struct impl));
-	if (impl == NULL)
+	if (impl == NULL) {
-		return NULL;
+		res = -errno;
+		goto error_cleanup;
+	}
 
 	this = &impl->this;
 	impl->properties = properties;
@@ -79,15 +81,15 @@ struct pw_loop *pw_loop_new(struct pw_properties *properties)
 			n_support, support);
 	if (impl->system_handle == NULL) {
 		res = -errno;
-		pw_log_error("can't make system handle");
+		pw_log_error("can't make system handle: %m");
-		goto out_free;
+		goto error_free;
 	}
 
         if ((res = spa_handle_get_interface(impl->system_handle,
 					    SPA_TYPE_INTERFACE_System,
 					    &iface)) < 0) {
-                pw_log_error("can't get System interface %d\n", res);
+                pw_log_error("can't get System interface %s", spa_strerror(res));
-                goto out_free_system;
+                goto error_unload_system;
 	}
 	this->system = iface;
 
@@ -105,14 +107,14 @@ struct pw_loop *pw_loop_new(struct pw_properties *properties)
 	if (impl->loop_handle == NULL) {
 		res = -errno;
 		pw_log_error("can't make loop handle: %m");
-		goto out_free_system;
+		goto error_unload_system;
 	}
 
         if ((res = spa_handle_get_interface(impl->loop_handle,
 					    SPA_TYPE_INTERFACE_Loop,
 					    &iface)) < 0) {
                 fprintf(stderr, "can't get Loop interface %d\n", res);
-                goto out_free_loop;
+                goto error_unload_loop;
         }
 	this->loop = iface;
 
@@ -120,7 +122,7 @@ struct pw_loop *pw_loop_new(struct pw_properties *properties)
 					    SPA_TYPE_INTERFACE_LoopControl,
 					    &iface)) < 0) {
                 fprintf(stderr, "can't get LoopControl interface %d\n", res);
-                goto out_free_loop;
+                goto error_unload_loop;
         }
 	this->control = iface;
 
@@ -128,18 +130,21 @@ struct pw_loop *pw_loop_new(struct pw_properties *properties)
 					    SPA_TYPE_INTERFACE_LoopUtils,
 					    &iface)) < 0) {
                 fprintf(stderr, "can't get LoopUtils interface %d\n", res);
-                goto out_free_loop;
+                goto error_unload_loop;
         }
 	this->utils = iface;
 
 	return this;
 
-      out_free_loop:
+error_unload_loop:
 	pw_unload_spa_handle(impl->loop_handle);
-      out_free_system:
+error_unload_system:
 	pw_unload_spa_handle(impl->system_handle);
-      out_free:
+error_free:
 	free(impl);
+error_cleanup:
+	if (properties)
+		pw_properties_free(properties);
 	errno = -res;
 	return NULL;
 }

src/pipewire/main-loop.c

@@ -45,31 +45,37 @@ struct pw_main_loop *pw_main_loop_new(struct pw_properties *properties)
 	int res;
 
 	this = calloc(1, sizeof(struct pw_main_loop));
-	if (this == NULL)
+	if (this == NULL) {
-		return NULL;
+		res = -errno;
+		goto error_cleanup;
+	}
 
 	pw_log_debug("main-loop %p: new", this);
 
 	this->loop = pw_loop_new(properties);
+	properties = NULL;
 	if (this->loop == NULL) {
 		res = -errno;
-		goto no_loop;
+		goto error_free;
 	}
 
         this->event = pw_loop_add_event(this->loop, do_stop, this);
 	if (this->event == NULL) {
 		res = -errno;
-		goto no_event;
+		goto error_free_loop;
 	}
 
 	spa_hook_list_init(&this->listener_list);
 
 	return this;
 
-      no_event:
+error_free_loop:
 	pw_loop_destroy(this->loop);
-      no_loop:
+error_free:
 	free(this);
+error_cleanup:
+	if (properties)
+		pw_properties_free(properties);
 	errno = -res;
 	return NULL;
 }
@@ -113,10 +119,10 @@ struct pw_loop * pw_main_loop_get_loop(struct pw_main_loop *loop)
  * \memberof pw_main_loop
  */
 SPA_EXPORT
-void pw_main_loop_quit(struct pw_main_loop *loop)
+int pw_main_loop_quit(struct pw_main_loop *loop)
 {
 	pw_log_debug("main-loop %p: quit", loop);
-	pw_loop_signal_event(loop->loop, loop->event);
+	return pw_loop_signal_event(loop->loop, loop->event);
 }
 
 /** Start a main loop
@@ -128,14 +134,19 @@ void pw_main_loop_quit(struct pw_main_loop *loop)
  * \memberof pw_main_loop
  */
 SPA_EXPORT
-void pw_main_loop_run(struct pw_main_loop *loop)
+int pw_main_loop_run(struct pw_main_loop *loop)
 {
+	int res = 0;
+
 	pw_log_debug("main-loop %p: run", loop);
 
 	loop->running = true;
 	pw_loop_enter(loop->loop);
 	while (loop->running) {
-		pw_loop_iterate(loop->loop, -1);
+		if ((res = pw_loop_iterate(loop->loop, -1)) < 0)
+			pw_log_warn("main-loop %p: iterate error %d (%s)",
+					loop, res, spa_strerror(res));
 	}
 	pw_loop_leave(loop->loop);
+	return res;
 }

src/pipewire/main-loop.h

@@ -67,10 +67,10 @@ struct pw_loop * pw_main_loop_get_loop(struct pw_main_loop *loop);
 void pw_main_loop_destroy(struct pw_main_loop *loop);
 
 /** Run a main loop. This blocks until \ref pw_main_loop_quit is called */
-void pw_main_loop_run(struct pw_main_loop *loop);
+int pw_main_loop_run(struct pw_main_loop *loop);
 
 /** Quit a main loop */
-void pw_main_loop_quit(struct pw_main_loop *loop);
+int pw_main_loop_quit(struct pw_main_loop *loop);
 
 #ifdef __cplusplus
 }

src/pipewire/module.c

@@ -209,26 +209,25 @@ pw_module_load(struct pw_core *core,
 	}
 
 	if (filename == NULL)
-		goto not_found;
+		goto error_not_found;
 
 	pw_log_debug("trying to load module: %s (%s)", name, filename);
 
 	hnd = dlopen(filename, RTLD_NOW | RTLD_LOCAL);
-
 	if (hnd == NULL)
-		goto open_failed;
+		goto error_open_failed;
 
 	if ((init_func = dlsym(hnd, PIPEWIRE_SYMBOL_MODULE_INIT)) == NULL)
-		goto no_pw_module;
+		goto error_no_pw_module;
-
-	impl = calloc(1, sizeof(struct impl));
-	if (impl == NULL)
-		goto no_mem;
 
 	if (properties == NULL)
 		properties = pw_properties_new(NULL, NULL);
 	if (properties == NULL)
-		goto no_mem;
+		goto error_no_mem;
+
+	impl = calloc(1, sizeof(struct impl));
+	if (impl == NULL)
+		goto error_no_mem;
 
 	impl->hnd = hnd;
 
@@ -245,8 +244,6 @@ pw_module_load(struct pw_core *core,
 	this->info.args = args ? strdup(args) : NULL;
 	this->info.props = &this->properties->dict;
 
-	spa_list_append(&core->module_list, &this->link);
-
 	this->global = pw_global_new(core,
 				     PW_TYPE_INTERFACE_Module,
 				     PW_VERSION_MODULE_PROXY,
@@ -257,13 +254,14 @@ pw_module_load(struct pw_core *core,
 				     this);
 
 	if (this->global == NULL)
-		goto no_global;
+		goto error_no_global;
 
+	spa_list_append(&core->module_list, &this->link);
 	pw_global_add_listener(this->global, &this->global_listener, &global_events, this);
 	this->info.id = this->global->id;
 
 	if ((res = init_func(this, args)) < 0)
-		goto init_failed;
+		goto error_init_failed;
 
 	pw_global_register(this->global, owner, parent);
 
@@ -271,26 +269,40 @@ pw_module_load(struct pw_core *core,
 
 	return this;
 
-      not_found:
+error_not_found:
+	res = -ENOENT;
 	pw_log_error("No module \"%s\" was found", name);
-	return NULL;
+	goto error_cleanup;
-      open_failed:
+error_open_failed:
+	res = -ENOENT;
 	pw_log_error("Failed to open module: \"%s\" %s", filename, dlerror());
-	free(filename);
+	goto error_free_filename;
-	return NULL;
+error_no_pw_module:
-      no_mem:
+	res = -ENOSYS;
-      no_pw_module:
 	pw_log_error("\"%s\": is not a pipewire module", filename);
-	dlclose(hnd);
+	goto error_close;
-	free(filename);
+error_no_mem:
-	return NULL;
+	res = -errno;
-      no_global:
+	pw_log_error("can't allocate module: %m");
-	pw_log_error("\"%s\": failed to create global", filename);
+	goto error_close;
-	pw_module_destroy(this);
+error_no_global:
-	return NULL;
+	res = -errno;
-      init_failed:
+	pw_log_error("\"%s\": failed to create global: %m", filename);
+	goto error_free_module;
+error_init_failed:
 	pw_log_error("\"%s\": failed to initialize: %s", filename, spa_strerror(res));
+	goto error_free_module;
+
+error_free_module:
 	pw_module_destroy(this);
+error_close:
+	dlclose(hnd);
+error_free_filename:
+	free(filename);
+error_cleanup:
+	if (properties)
+		pw_properties_free(properties);
+	errno = -res;
 	return NULL;
 }
 
@@ -306,9 +318,8 @@ void pw_module_destroy(struct pw_module *module)
 	pw_log_debug("module %p: destroy", module);
 	pw_module_emit_destroy(module);
 
-	spa_list_remove(&module->link);
-
 	if (module->global) {
+		spa_list_remove(&module->link);
 		spa_hook_remove(&module->global_listener);
 		pw_global_destroy(module->global);
 	}

src/pipewire/node.c

@@ -458,6 +458,7 @@ global_bind(void *_data, struct pw_client *client, uint32_t permissions,
 	this->info.change_mask = PW_NODE_CHANGE_MASK_ALL;
 	pw_node_resource_info(resource, &this->info);
 	this->info.change_mask = 0;
+
 	return 0;
 
 error_resource:
@@ -491,7 +492,7 @@ int pw_node_register(struct pw_node *this,
 	pw_log_debug("node %p: register", this);
 
 	if (this->registered)
-		return -EEXIST;
+		goto error_existed;
 
 	if (properties == NULL)
 		properties = pw_properties_new(NULL, NULL);
@@ -506,9 +507,6 @@ int pw_node_register(struct pw_node *this,
 	if ((str = pw_properties_get(this->properties, PW_KEY_NODE_SESSION)) != NULL)
 		pw_properties_set(properties, PW_KEY_NODE_SESSION, str);
 
-	spa_list_append(&core->node_list, &this->link);
-	this->registered = true;
-
 	this->global = pw_global_new(core,
 				     PW_TYPE_INTERFACE_Node,
 				     PW_VERSION_NODE_PROXY,
@@ -518,6 +516,9 @@ int pw_node_register(struct pw_node *this,
 	if (this->global == NULL)
 		return -errno;
 
+	spa_list_append(&core->node_list, &this->link);
+	this->registered = true;
+
 	this->info.id = this->global->id;
 	this->rt.activation->position.clock.id = this->info.id;
 	pw_properties_setf(this->properties, PW_KEY_NODE_ID, "%d", this->info.id);
@@ -535,6 +536,11 @@ int pw_node_register(struct pw_node *this,
 				 pw_properties_copy(port->properties));
 
 	return 0;
+
+error_existed:
+	if (properties)
+		pw_properties_free(properties);
+	return -EEXIST;
 }
 
 SPA_EXPORT

src/pipewire/port.c

@@ -356,7 +356,7 @@ struct pw_port *pw_port_new(enum pw_direction direction,
 
 	if (properties == NULL) {
 		res = -errno;
-		goto no_mem;
+		goto error_no_mem;
 	}
 
 	if (SPA_FLAG_CHECK(info->flags, SPA_PORT_FLAG_PHYSICAL))
@@ -408,7 +408,7 @@ struct pw_port *pw_port_new(enum pw_direction direction,
 
 	return this;
 
-       no_mem:
+error_no_mem:
 	pw_log_warn("port %p: new failed", impl);
 	free(impl);
 	errno = -res;

src/pipewire/remote.c

@@ -163,8 +163,10 @@ struct pw_remote *pw_remote_new(struct pw_core *core,
 	int res;
 
 	impl = calloc(1, sizeof(struct remote) + user_data_size);
-	if (impl == NULL)
+	if (impl == NULL) {
-		return NULL;
+		res = -errno;
+		goto exit_cleanup;
+	}
 
 	this = &impl->this;
 	pw_log_debug("remote %p: new", impl);
@@ -177,7 +179,7 @@ struct pw_remote *pw_remote_new(struct pw_core *core,
 	if (properties == NULL)
 		properties = pw_properties_new(NULL, NULL);
 	if (properties == NULL)
-		goto no_mem;
+		goto error_properties;
 
 	pw_fill_remote_properties(core, properties);
 	this->properties = properties;
@@ -193,19 +195,23 @@ struct pw_remote *pw_remote_new(struct pw_core *core,
 
 	if ((protocol_name = pw_properties_get(properties, PW_KEY_PROTOCOL)) == NULL) {
 		if (pw_module_load(core, "libpipewire-module-protocol-native",
-					NULL, NULL, NULL, NULL) == NULL)
+					NULL, NULL, NULL, NULL) == NULL) {
-			goto no_protocol;
+			res = -errno;
+			goto error_protocol;
+		}
 
 		protocol_name = PW_TYPE_INFO_PROTOCOL_Native;
 	}
 
 	protocol = pw_core_find_protocol(core, protocol_name);
-	if (protocol == NULL)
+	if (protocol == NULL) {
-		goto no_protocol;
+		res = -ENOTSUP;
+		goto error_protocol;
+	}
 
 	this->conn = pw_protocol_new_client(protocol, this, properties);
 	if (this->conn == NULL)
-		goto no_connection;
+		goto error_connection;
 
 	pw_module_load(core, "libpipewire-module-rtkit", NULL, NULL, NULL, NULL);
 	pw_module_load(core, "libpipewire-module-client-node", NULL, NULL, NULL, NULL);
@@ -214,23 +220,23 @@ struct pw_remote *pw_remote_new(struct pw_core *core,
 
 	return this;
 
-      no_mem:
+error_properties:
 	res = -errno;
-	pw_log_error("no memory");
+	pw_log_error("can't create properties: %m");
-	goto exit;
+	goto exit_free;
-      no_protocol:
+error_protocol:
-	res = -errno;
+	pw_log_error("can't load native protocol: %s", spa_strerror(res));
-	pw_log_error("can't load native protocol: %m");
+	goto exit_free;
-	goto exit_free_props;
+error_connection:
-      no_connection:
 	res = -errno;
 	pw_log_error("can't create new native protocol connection: %m");
-	goto exit_free_props;
+	goto exit_free;
 
-      exit_free_props:
+exit_free:
-	pw_properties_free(properties);
-      exit:
 	free(impl);
+exit_cleanup:
+	if (properties)
+		pw_properties_free(properties);
 	errno = -res;
 	return NULL;
 }
@@ -462,33 +468,32 @@ struct pw_proxy *pw_remote_export(struct pw_remote *remote,
 
 	if (remote->core_proxy == NULL) {
 		res = -ENETDOWN;
-		goto no_core_proxy;
+		goto error_core_proxy;
 	}
 
 	t = pw_core_find_export_type(remote->core, type);
 	if (t == NULL) {
 		res = -EPROTO;
-		goto no_export_type;
+		goto error_export_type;
 	}
 
 	proxy = t->func(remote, type, props, object, user_data_size);
         if (proxy == NULL) {
 		res = -errno;
-		goto proxy_failed;
+		goto error_proxy_failed;
 	}
 	return proxy;
 
-    no_core_proxy:
+error_core_proxy:
 	pw_log_error("no core proxy: %s", spa_strerror(res));
-	goto out;
+	goto exit;
-    no_export_type:
+error_export_type:
 	pw_log_error("can't export type %d: %s", type, spa_strerror(res));
-	goto out;
+	goto exit;
-    proxy_failed:
+error_proxy_failed:
 	pw_log_error("failed to create proxy: %s", spa_strerror(res));
-	goto out;
+	goto exit;
-
+exit:
-    out:
 	errno = -res;
 	return NULL;
 }

src/pipewire/stream.c

@@ -1055,8 +1055,10 @@ struct pw_stream * pw_stream_new(struct pw_remote *remote, const char *name,
 	int res;
 
 	impl = calloc(1, sizeof(struct stream));
-	if (impl == NULL)
+	if (impl == NULL) {
-		return NULL;
+		res = -errno;
+		goto error_cleanup;
+	}
 
 	this = &impl->this;
 	pw_log_debug("stream %p: new \"%s\"", impl, name);
@@ -1067,8 +1069,8 @@ struct pw_stream * pw_stream_new(struct pw_remote *remote, const char *name,
 		pw_properties_set(props, PW_KEY_MEDIA_NAME, name);
 	}
 	if (props == NULL) {
-		res = errno;
+		res = -errno;
-		goto no_mem;
+		goto error_properties;
 	}
 
 	if (pw_properties_get(props, PW_KEY_NODE_NAME) == NULL) {
@@ -1109,8 +1111,11 @@ struct pw_stream * pw_stream_new(struct pw_remote *remote, const char *name,
 
 	return this;
 
-      no_mem:
+error_properties:
 	free(impl);
+error_cleanup:
+	if (props)
+		pw_properties_free(props);
 	errno = -res;
 	return NULL;
 }
@@ -1135,7 +1140,7 @@ pw_stream_new_simple(struct pw_loop *loop,
 	stream = pw_stream_new(remote, name, props);
 	if (stream == NULL) {
 		res = -errno;
-		goto cleanup;
+		goto error_cleanup;
 	}
 
 	impl = SPA_CONTAINER_OF(stream, struct stream, this);
@@ -1148,7 +1153,7 @@ pw_stream_new_simple(struct pw_loop *loop,
 
 	return stream;
 
-      cleanup:
+error_cleanup:
 	pw_core_destroy(core);
 	errno = -res;
 	return NULL;

src/pipewire/thread-loop.c

@@ -113,7 +113,7 @@ struct pw_thread_loop *pw_thread_loop_new(struct pw_loop *loop,
 
 	this = calloc(1, sizeof(struct pw_thread_loop));
 	if (this == NULL)
-		goto no_mem;
+		return NULL;
 
 	pw_log_debug("thread-loop %p: new", this);
 
@@ -151,7 +151,6 @@ struct pw_thread_loop *pw_thread_loop_new(struct pw_loop *loop,
 	free(this->name);
 	free(this);
 	errno = -res;
-      no_mem:
 	return NULL;
 }
 
@@ -203,7 +202,8 @@ static void *do_loop(void *user_data)
 
 	while (this->running) {
 		if ((res = pw_loop_iterate(this->loop, -1)) < 0)
-			pw_log_warn("thread-loop %p: iterate error %d", this, res);
+			pw_log_warn("thread-loop %p: iterate error %d (%s)",
+					this, res, spa_strerror(res));
 	}
 	pw_log_debug("thread-loop %p: leave thread", this);
 	pw_loop_leave(this->loop);

src/pipewire/work-queue.c

@@ -107,16 +107,17 @@ struct pw_work_queue *pw_work_queue_new(struct pw_loop *loop)
 	this->loop = loop;
 
 	this->wakeup = pw_loop_add_event(this->loop, process_work_queue, this);
-	if (this->wakeup == NULL)
+	if (this->wakeup == NULL) {
-		goto out_free;
+		res = -errno;
+		goto error_free;
+	}
 
 	spa_list_init(&this->work_list);
 	spa_list_init(&this->free_list);
 
 	return this;
 
-      out_free:
+error_free:
-	res = -errno;
 	free(this);
 	errno = -res;
 	return NULL;