From d1241e2c1c837ff59186f0d10c8261239ba80390 Mon Sep 17 00:00:00 2001 From: Wim Taymans Date: Wed, 19 Jun 2019 18:15:04 +0200 Subject: [PATCH] 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 | 8 ++-- spa/plugins/support/loop.c | 10 +++-- src/pipewire/client.c | 13 +++++-- src/pipewire/device.c | 14 +++++-- src/pipewire/factory.c | 13 +++++-- src/pipewire/global.c | 16 ++++++-- src/pipewire/link.c | 69 +++++++++++++++++++-------------- src/pipewire/loop.c | 31 ++++++++------- src/pipewire/main-loop.c | 31 ++++++++++----- src/pipewire/main-loop.h | 4 +- src/pipewire/module.c | 71 ++++++++++++++++++++-------------- src/pipewire/node.c | 14 +++++-- src/pipewire/port.c | 4 +- src/pipewire/remote.c | 67 +++++++++++++++++--------------- src/pipewire/stream.c | 19 +++++---- src/pipewire/thread-loop.c | 6 +-- src/pipewire/work-queue.c | 9 +++-- 17 files changed, 241 insertions(+), 158 deletions(-) diff --git a/spa/include/spa/support/loop.h b/spa/include/spa/support/loop.h index 6901859b8..272508cd0 100644 --- a/spa/include/spa/support/loop.h +++ b/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__) diff --git a/spa/plugins/support/loop.c b/spa/plugins/support/loop.c index 2dcc692ae..00803222f 100644 --- a/spa/plugins/support/loop.c +++ b/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) diff --git a/src/pipewire/client.c b/src/pipewire/client.c index ae5e0973a..5a902a656 100644 --- a/src/pipewire/client.c +++ b/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 diff --git a/src/pipewire/device.c b/src/pipewire/device.c index 23c018836..277f03570 100644 --- a/src/pipewire/device.c +++ b/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) diff --git a/src/pipewire/factory.c b/src/pipewire/factory.c index f4c8618d3..7cda0336e 100644 --- a/src/pipewire/factory.c +++ b/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 diff --git a/src/pipewire/global.c b/src/pipewire/global.c index 3026d8b2e..77abd49db 100644 --- a/src/pipewire/global.c +++ b/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) - return NULL; + if (impl == 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; diff --git a/src/pipewire/link.c b/src/pipewire/link.c index 87d6ea342..13c9d4857 100644 --- a/src/pipewire/link.c +++ b/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: - pw_log_error("can't set io %d (%s)", res, spa_strerror(res)); - errno = -res; - return NULL; - same_ports: +error_same_ports: + res = -EINVAL; pw_log_error("can't link the same ports"); - errno = EINVAL; - return NULL; - wrong_direction: + goto error_exit; +error_wrong_direction: + res = -EINVAL; pw_log_error("ports have wrong direction"); - errno = EINVAL; - return NULL; - link_exists: + goto error_exit; +error_link_exists: + res = -EEXIST; pw_log_error("link already exists"); - errno = EEXIST; - return NULL; - link_not_allowed: + goto error_exit; +error_link_not_allowed: + res = -EPERM; pw_log_error("link not allowed"); - errno = EPERM; - return NULL; - no_mem: - pw_log_error("no memory"); + goto error_exit; +error_no_mem: + res = -errno; + 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 diff --git a/src/pipewire/loop.c b/src/pipewire/loop.c index 867c15df9..9a4a7c85c 100644 --- a/src/pipewire/loop.c +++ b/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) - return NULL; + if (impl == 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"); - goto out_free; + pw_log_error("can't make system handle: %m"); + 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); - goto out_free_system; + pw_log_error("can't get System interface %s", spa_strerror(res)); + 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; } diff --git a/src/pipewire/main-loop.c b/src/pipewire/main-loop.c index 410694fea..83633729c 100644 --- a/src/pipewire/main-loop.c +++ b/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) - return NULL; + if (this == 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; } diff --git a/src/pipewire/main-loop.h b/src/pipewire/main-loop.h index 6a72508d8..b3976742d 100644 --- a/src/pipewire/main-loop.h +++ b/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 } diff --git a/src/pipewire/module.c b/src/pipewire/module.c index 9d685375c..b36adf86e 100644 --- a/src/pipewire/module.c +++ b/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; - - impl = calloc(1, sizeof(struct impl)); - if (impl == NULL) - goto no_mem; + goto error_no_pw_module; 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; - open_failed: + goto error_cleanup; +error_open_failed: + res = -ENOENT; pw_log_error("Failed to open module: \"%s\" %s", filename, dlerror()); - free(filename); - return NULL; - no_mem: - no_pw_module: + goto error_free_filename; +error_no_pw_module: + res = -ENOSYS; pw_log_error("\"%s\": is not a pipewire module", filename); - dlclose(hnd); - free(filename); - return NULL; - no_global: - pw_log_error("\"%s\": failed to create global", filename); - pw_module_destroy(this); - return NULL; - init_failed: + goto error_close; +error_no_mem: + res = -errno; + pw_log_error("can't allocate module: %m"); + goto error_close; +error_no_global: + res = -errno; + 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); } diff --git a/src/pipewire/node.c b/src/pipewire/node.c index b761ec784..491ea92d8 100644 --- a/src/pipewire/node.c +++ b/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 diff --git a/src/pipewire/port.c b/src/pipewire/port.c index 07bcd429f..20ac04aef 100644 --- a/src/pipewire/port.c +++ b/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; diff --git a/src/pipewire/remote.c b/src/pipewire/remote.c index d8871781d..42bd0ebc6 100644 --- a/src/pipewire/remote.c +++ b/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) - return NULL; + if (impl == 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) - goto no_protocol; + NULL, NULL, NULL, NULL) == NULL) { + res = -errno; + goto error_protocol; + } protocol_name = PW_TYPE_INFO_PROTOCOL_Native; } protocol = pw_core_find_protocol(core, protocol_name); - if (protocol == NULL) - goto no_protocol; + if (protocol == NULL) { + 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"); - goto exit; - no_protocol: - res = -errno; - pw_log_error("can't load native protocol: %m"); - goto exit_free_props; - no_connection: + pw_log_error("can't create properties: %m"); + goto exit_free; +error_protocol: + pw_log_error("can't load native protocol: %s", spa_strerror(res)); + goto exit_free; +error_connection: res = -errno; pw_log_error("can't create new native protocol connection: %m"); - goto exit_free_props; + goto exit_free; - exit_free_props: - pw_properties_free(properties); - exit: +exit_free: 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; - no_export_type: + goto exit; +error_export_type: pw_log_error("can't export type %d: %s", type, spa_strerror(res)); - goto out; - proxy_failed: + goto exit; +error_proxy_failed: pw_log_error("failed to create proxy: %s", spa_strerror(res)); - goto out; - - out: + goto exit; +exit: errno = -res; return NULL; } diff --git a/src/pipewire/stream.c b/src/pipewire/stream.c index f48d5ee0a..ed743e7a3 100644 --- a/src/pipewire/stream.c +++ b/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) - return NULL; + if (impl == 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; - goto no_mem; + res = -errno; + 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; diff --git a/src/pipewire/thread-loop.c b/src/pipewire/thread-loop.c index 8f5a0406a..1006566be 100644 --- a/src/pipewire/thread-loop.c +++ b/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); diff --git a/src/pipewire/work-queue.c b/src/pipewire/work-queue.c index d1d69d99f..a83d1ac79 100644 --- a/src/pipewire/work-queue.c +++ b/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) - goto out_free; + if (this->wakeup == NULL) { + res = -errno; + goto error_free; + } spa_list_init(&this->work_list); spa_list_init(&this->free_list); return this; - out_free: - res = -errno; +error_free: free(this); errno = -res; return NULL;