From 91755950dda1d951ad861ef509603d56dfe862ac Mon Sep 17 00:00:00 2001 From: Wim Taymans Date: Wed, 6 May 2026 14:20:52 +0200 Subject: [PATCH] spa: improve error handling Use impl_clear to clean up partially allocated handles. Make sure we only clean up the initialized parts. --- spa/plugins/bluez5/media-sink.c | 33 +++++++++++++------------------ spa/plugins/bluez5/media-source.c | 12 +++++------ spa/plugins/support/node-driver.c | 21 ++++++++++++-------- 3 files changed, 33 insertions(+), 33 deletions(-) diff --git a/spa/plugins/bluez5/media-sink.c b/spa/plugins/bluez5/media-sink.c index 70ce7b930..000841775 100644 --- a/spa/plugins/bluez5/media-sink.c +++ b/spa/plugins/bluez5/media-sink.c @@ -2523,10 +2523,13 @@ static int impl_clear(struct spa_handle *handle) this->codec->clear_props(this->codec_props); if (this->transport) spa_hook_remove(&this->transport_listener); - spa_system_close(this->data_system, this->timerfd); - spa_system_close(this->data_system, this->flush_timerfd); - if (this->codec->kind == MEDIA_CODEC_ASHA) { - spa_system_close(this->data_system, this->asha->timerfd); + if (this->timerfd > 0) + spa_system_close(this->data_system, this->timerfd); + if (this->flush_timerfd > 0) + spa_system_close(this->data_system, this->flush_timerfd); + if (this->codec->kind == MEDIA_CODEC_ASHA && this->asha) { + if (this->asha->timerfd > 0) + spa_system_close(this->data_system, this->asha->timerfd); free(this->asha); } return 0; @@ -2674,38 +2677,30 @@ impl_init(const struct spa_handle_factory *factory, if ((res = spa_system_timerfd_create(this->data_system, CLOCK_MONOTONIC, SPA_FD_CLOEXEC | SPA_FD_NONBLOCK)) < 0) - goto error_remove_listener; + goto error; this->timerfd = res; if ((res = spa_system_timerfd_create(this->data_system, CLOCK_MONOTONIC, SPA_FD_CLOEXEC | SPA_FD_NONBLOCK)) < 0) - goto error_close_timerfd; + goto error; this->flush_timerfd = res; if (this->codec->kind == MEDIA_CODEC_ASHA) { this->asha = calloc(1, sizeof(struct spa_bt_asha)); if (this->asha == NULL) { res = -errno; - goto error_close_flush_timerfd; + goto error; } + this->asha->timerfd = -1; if ((res = spa_system_timerfd_create(this->data_system, CLOCK_MONOTONIC, SPA_FD_CLOEXEC | SPA_FD_NONBLOCK)) < 0) - goto error_free_asha; + goto error; this->asha->timerfd = res; } return 0; -error_free_asha: - free(this->asha); - this->asha = NULL; -error_close_flush_timerfd: - spa_system_close(this->data_system, this->flush_timerfd); -error_close_timerfd: - spa_system_close(this->data_system, this->timerfd); -error_remove_listener: - spa_hook_remove(&this->transport_listener); - if (this->codec_props && this->codec->clear_props) - this->codec->clear_props(this->codec_props); +error: + impl_clear(handle); return res; } diff --git a/spa/plugins/bluez5/media-source.c b/spa/plugins/bluez5/media-source.c index a730254e2..536ca284f 100644 --- a/spa/plugins/bluez5/media-source.c +++ b/spa/plugins/bluez5/media-source.c @@ -2112,7 +2112,8 @@ static int impl_clear(struct spa_handle *handle) this->codec->clear_props(this->codec_props); if (this->transport) spa_hook_remove(&this->transport_listener); - spa_system_close(this->data_system, this->timerfd); + if (this->timerfd > 0) + spa_system_close(this->data_system, this->timerfd); spa_bt_decode_buffer_clear(&port->buffer); return 0; } @@ -2148,6 +2149,7 @@ impl_init(const struct spa_handle_factory *factory, this->data_loop = spa_support_find(support, n_support, SPA_TYPE_INTERFACE_DataLoop); this->data_system = spa_support_find(support, n_support, SPA_TYPE_INTERFACE_DataSystem); this->loop_utils = spa_support_find(support, n_support, SPA_TYPE_INTERFACE_LoopUtils); + this->timerfd = -1; spa_log_topic_init(this->log, &log_topic); @@ -2271,7 +2273,7 @@ impl_init(const struct spa_handle_factory *factory, if ((res = spa_system_timerfd_create(this->data_system, CLOCK_MONOTONIC, SPA_FD_CLOEXEC | SPA_FD_NONBLOCK)) < 0) - goto error_remove_listener; + goto error; this->timerfd = res; this->node_latency = 512; @@ -2282,10 +2284,8 @@ impl_init(const struct spa_handle_factory *factory, return 0; -error_remove_listener: - spa_hook_remove(&this->transport_listener); - if (this->codec_props && this->codec->clear_props) - this->codec->clear_props(this->codec_props); +error: + impl_clear(handle); return res; } diff --git a/spa/plugins/support/node-driver.c b/spa/plugins/support/node-driver.c index 693e2da5f..578391120 100644 --- a/spa/plugins/support/node-driver.c +++ b/spa/plugins/support/node-driver.c @@ -927,9 +927,10 @@ static int impl_clear(struct spa_handle *handle) this = (struct impl *) handle; - spa_loop_locked(this->data_loop, do_remove_timer, 0, NULL, 0, this); - spa_system_close(this->data_system, this->timer_source.fd); - + if (this->timer_source.data) { + spa_loop_locked(this->data_loop, do_remove_timer, 0, NULL, 0, this); + spa_system_close(this->data_system, this->timer_source.fd); + } if (this->clock_fd != -1) close(this->clock_fd); @@ -952,6 +953,7 @@ impl_init(const struct spa_handle_factory *factory, { struct impl *this; uint32_t i; + int res; spa_return_val_if_fail(factory != NULL, -EINVAL); spa_return_val_if_fail(handle != NULL, -EINVAL); @@ -1038,19 +1040,22 @@ impl_init(const struct spa_handle_factory *factory, this->nsec_offset.offset = get_nsec_offset(this, NULL); this->nsec_offset.err = 0; + if ((res = spa_system_timerfd_create(this->data_system, + this->timer_clockid, SPA_FD_CLOEXEC | SPA_FD_NONBLOCK)) < 0) + goto error; + this->timer_source.func = on_timeout; this->timer_source.data = this; - this->timer_source.fd = spa_system_timerfd_create(this->data_system, - this->timer_clockid, SPA_FD_CLOEXEC | SPA_FD_NONBLOCK); - if (this->timer_source.fd < 0) - return this->timer_source.fd; - + this->timer_source.fd = res; this->timer_source.mask = SPA_IO_IN; this->timer_source.rmask = 0; spa_loop_add_source(this->data_loop, &this->timer_source); return 0; +error: + impl_clear(handle); + return res; } static const struct spa_interface_info impl_interfaces[] = {