mirror of
https://gitlab.freedesktop.org/pipewire/pipewire.git
synced 2026-06-09 03:01:53 -04:00
modules: avoid double fd close
Now that loop_add_io, avoid double free on failures. Try to use spa_autoclose and spa_steal_fd to make the error paths easier.
This commit is contained in:
parent
a9f62579ba
commit
894e97aaa5
5 changed files with 60 additions and 76 deletions
|
|
@ -14,6 +14,7 @@
|
|||
|
||||
#include <spa/support/cpu.h>
|
||||
#include <spa/debug/mem.h>
|
||||
#include <spa/utils/cleanup.h>
|
||||
#include <spa/utils/result.h>
|
||||
|
||||
#include <pipewire/pipewire.h>
|
||||
|
|
@ -148,12 +149,12 @@ static int load_filter(int fd, uint16_t eth, const uint8_t dest[6], const uint8_
|
|||
|
||||
static int raw_make_socket(struct server *server, uint16_t type, const uint8_t mac[6])
|
||||
{
|
||||
int fd, res;
|
||||
int res;
|
||||
struct ifreq req;
|
||||
struct packet_mreq mreq;
|
||||
struct sockaddr_ll sll;
|
||||
|
||||
fd = socket(AF_PACKET, SOCK_RAW | SOCK_CLOEXEC | SOCK_NONBLOCK, htons(ETH_P_ALL));
|
||||
spa_autoclose int fd = socket(AF_PACKET, SOCK_RAW | SOCK_CLOEXEC | SOCK_NONBLOCK, htons(ETH_P_ALL));
|
||||
if (fd < 0) {
|
||||
pw_log_error("socket() failed: %m");
|
||||
return -errno;
|
||||
|
|
@ -164,7 +165,7 @@ static int raw_make_socket(struct server *server, uint16_t type, const uint8_t m
|
|||
if (ioctl(fd, SIOCGIFINDEX, &req) < 0) {
|
||||
res = -errno;
|
||||
pw_log_error("SIOCGIFINDEX %s failed: %m", server->ifname);
|
||||
goto error_close;
|
||||
return res;
|
||||
}
|
||||
server->ifindex = req.ifr_ifindex;
|
||||
|
||||
|
|
@ -173,7 +174,7 @@ static int raw_make_socket(struct server *server, uint16_t type, const uint8_t m
|
|||
if (ioctl(fd, SIOCGIFHWADDR, &req) < 0) {
|
||||
res = -errno;
|
||||
pw_log_error("SIOCGIFHWADDR %s failed: %m", server->ifname);
|
||||
goto error_close;
|
||||
return res;
|
||||
}
|
||||
memcpy(server->mac_addr, req.ifr_hwaddr.sa_data, sizeof(server->mac_addr));
|
||||
|
||||
|
|
@ -193,7 +194,7 @@ static int raw_make_socket(struct server *server, uint16_t type, const uint8_t m
|
|||
if (bind(fd, (struct sockaddr *) &sll, sizeof(sll)) < 0) {
|
||||
res = -errno;
|
||||
pw_log_error("bind() failed: %m");
|
||||
goto error_close;
|
||||
return res;
|
||||
}
|
||||
|
||||
spa_zero(mreq);
|
||||
|
|
@ -206,17 +207,13 @@ static int raw_make_socket(struct server *server, uint16_t type, const uint8_t m
|
|||
&mreq, sizeof(mreq)) < 0) {
|
||||
res = -errno;
|
||||
pw_log_error("setsockopt(ADD_MEMBERSHIP) failed: %m");
|
||||
goto error_close;
|
||||
return res;
|
||||
}
|
||||
|
||||
if ((res = load_filter(fd, type, mac, server->mac_addr)) < 0)
|
||||
goto error_close;
|
||||
return res;
|
||||
|
||||
return fd;
|
||||
|
||||
error_close:
|
||||
close(fd);
|
||||
return res;
|
||||
return spa_steal_fd(fd);
|
||||
}
|
||||
|
||||
int avb_server_make_socket(struct server *server, uint16_t type, const uint8_t mac[6])
|
||||
|
|
@ -229,45 +226,40 @@ int avb_server_make_socket(struct server *server, uint16_t type, const uint8_t m
|
|||
static int raw_transport_setup(struct server *server)
|
||||
{
|
||||
struct impl *impl = server->impl;
|
||||
int fd, res;
|
||||
int res;
|
||||
static const uint8_t bmac[6] = AVB_BROADCAST_MAC;
|
||||
|
||||
fd = raw_make_socket(server, AVB_TSN_ETH, bmac);
|
||||
spa_autoclose int fd = raw_make_socket(server, AVB_TSN_ETH, bmac);
|
||||
if (fd < 0)
|
||||
return fd;
|
||||
|
||||
pw_log_info("0x%"PRIx64" %d", server->entity_id, server->ifindex);
|
||||
|
||||
server->source = pw_loop_add_io(impl->loop, fd, SPA_IO_IN, true, on_socket_data, server);
|
||||
server->source = pw_loop_add_io(impl->loop, spa_steal_fd(fd), SPA_IO_IN, true, on_socket_data, server);
|
||||
if (server->source == NULL) {
|
||||
res = -errno;
|
||||
pw_log_error("server %p: can't create server source: %m", impl);
|
||||
goto error_no_source;
|
||||
return res;
|
||||
}
|
||||
|
||||
if ((res = pw_timer_queue_add(impl->timer_queue, &server->timer,
|
||||
NULL, DEFAULT_INTERVAL_MS * SPA_NSEC_PER_MSEC,
|
||||
on_timer_event, server)) < 0) {
|
||||
pw_log_error("server %p: can't create timer: %s", impl, spa_strerror(res));
|
||||
goto error_no_timer;
|
||||
pw_loop_destroy_source(impl->loop, server->source);
|
||||
server->source = NULL;
|
||||
return res;
|
||||
}
|
||||
return 0;
|
||||
|
||||
error_no_timer:
|
||||
pw_loop_destroy_source(impl->loop, server->source);
|
||||
server->source = NULL;
|
||||
error_no_source:
|
||||
close(fd);
|
||||
return res;
|
||||
}
|
||||
|
||||
static int raw_stream_setup_socket(struct server *server, struct stream *stream)
|
||||
{
|
||||
int fd, res;
|
||||
int res;
|
||||
char buf[128];
|
||||
struct ifreq req;
|
||||
|
||||
fd = socket(AF_PACKET, SOCK_RAW | SOCK_CLOEXEC | SOCK_NONBLOCK, htons(ETH_P_ALL));
|
||||
spa_autoclose int fd = socket(AF_PACKET, SOCK_RAW | SOCK_CLOEXEC | SOCK_NONBLOCK, htons(ETH_P_ALL));
|
||||
if (fd < 0) {
|
||||
pw_log_error("socket() failed: %m");
|
||||
return -errno;
|
||||
|
|
@ -278,8 +270,7 @@ static int raw_stream_setup_socket(struct server *server, struct stream *stream)
|
|||
res = ioctl(fd, SIOCGIFINDEX, &req);
|
||||
if (res < 0) {
|
||||
pw_log_error("SIOCGIFINDEX %s failed: %m", server->ifname);
|
||||
res = -errno;
|
||||
goto error_close;
|
||||
return -errno;
|
||||
}
|
||||
|
||||
spa_zero(stream->sock_addr);
|
||||
|
|
@ -294,8 +285,7 @@ static int raw_stream_setup_socket(struct server *server, struct stream *stream)
|
|||
sizeof(stream->prio));
|
||||
if (res < 0) {
|
||||
pw_log_error("setsockopt(SO_PRIORITY %d) failed: %m", stream->prio);
|
||||
res = -errno;
|
||||
goto error_close;
|
||||
return -errno;
|
||||
}
|
||||
|
||||
txtime_cfg.clockid = CLOCK_TAI;
|
||||
|
|
@ -304,8 +294,7 @@ static int raw_stream_setup_socket(struct server *server, struct stream *stream)
|
|||
sizeof(txtime_cfg));
|
||||
if (res < 0) {
|
||||
pw_log_error("setsockopt(SO_TXTIME) failed: %m");
|
||||
res = -errno;
|
||||
goto error_close;
|
||||
return -errno;
|
||||
}
|
||||
} else {
|
||||
struct packet_mreq mreq;
|
||||
|
|
@ -313,8 +302,7 @@ static int raw_stream_setup_socket(struct server *server, struct stream *stream)
|
|||
res = bind(fd, (struct sockaddr *) &stream->sock_addr, sizeof(stream->sock_addr));
|
||||
if (res < 0) {
|
||||
pw_log_error("bind() failed: %m");
|
||||
res = -errno;
|
||||
goto error_close;
|
||||
return -errno;
|
||||
}
|
||||
|
||||
spa_zero(mreq);
|
||||
|
|
@ -329,15 +317,10 @@ static int raw_stream_setup_socket(struct server *server, struct stream *stream)
|
|||
|
||||
if (res < 0) {
|
||||
pw_log_error("setsockopt(ADD_MEMBERSHIP) failed: %m");
|
||||
res = -errno;
|
||||
goto error_close;
|
||||
return -errno;
|
||||
}
|
||||
}
|
||||
return fd;
|
||||
|
||||
error_close:
|
||||
close(fd);
|
||||
return res;
|
||||
return spa_steal_fd(fd);
|
||||
}
|
||||
|
||||
static ssize_t raw_stream_send(struct server *server, struct stream *stream,
|
||||
|
|
|
|||
|
|
@ -4,6 +4,8 @@
|
|||
|
||||
#include <unistd.h>
|
||||
|
||||
#include <spa/utils/cleanup.h>
|
||||
|
||||
#include <pipewire/pipewire.h>
|
||||
|
||||
#include "utils.h"
|
||||
|
|
@ -180,9 +182,9 @@ struct avb_mmrp_attribute *avb_mmrp_attribute_new(struct avb_mmrp *m,
|
|||
struct avb_mmrp *avb_mmrp_register(struct server *server)
|
||||
{
|
||||
struct mmrp *mmrp;
|
||||
int fd, res;
|
||||
int res;
|
||||
|
||||
fd = avb_server_make_socket(server, AVB_MMRP_ETH, mmrp_mac);
|
||||
spa_autoclose int fd = avb_server_make_socket(server, AVB_MMRP_ETH, mmrp_mac);
|
||||
if (fd < 0) {
|
||||
errno = -fd;
|
||||
return NULL;
|
||||
|
|
@ -190,26 +192,25 @@ struct avb_mmrp *avb_mmrp_register(struct server *server)
|
|||
mmrp = calloc(1, sizeof(*mmrp));
|
||||
if (mmrp == NULL) {
|
||||
res = -errno;
|
||||
goto error_close;
|
||||
goto error;
|
||||
}
|
||||
|
||||
mmrp->server = server;
|
||||
spa_list_init(&mmrp->attributes);
|
||||
|
||||
mmrp->source = pw_loop_add_io(server->impl->loop, fd, SPA_IO_IN, true, on_socket_data, mmrp);
|
||||
mmrp->source = pw_loop_add_io(server->impl->loop, spa_steal_fd(fd), SPA_IO_IN, true, on_socket_data, mmrp);
|
||||
if (mmrp->source == NULL) {
|
||||
res = -errno;
|
||||
pw_log_error("mmrp %p: can't create mmrp source: %m", mmrp);
|
||||
goto error_no_source;
|
||||
goto error_free;
|
||||
}
|
||||
avdecc_server_add_listener(server, &mmrp->server_listener, &server_events, mmrp);
|
||||
|
||||
return (struct avb_mmrp*)mmrp;
|
||||
|
||||
error_no_source:
|
||||
error_free:
|
||||
free(mmrp);
|
||||
error_close:
|
||||
close(fd);
|
||||
error:
|
||||
errno = -res;
|
||||
return NULL;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -7,6 +7,7 @@
|
|||
#include <unistd.h>
|
||||
|
||||
#include <spa/debug/mem.h>
|
||||
#include <spa/utils/cleanup.h>
|
||||
|
||||
#include <pipewire/pipewire.h>
|
||||
|
||||
|
|
@ -632,9 +633,9 @@ void avb_msrp_log_state(struct server *server, const char *label)
|
|||
struct avb_msrp *avb_msrp_register(struct server *server)
|
||||
{
|
||||
struct msrp *msrp;
|
||||
int fd, res;
|
||||
int res;
|
||||
|
||||
fd = avb_server_make_socket(server, AVB_MSRP_ETH, msrp_mac);
|
||||
spa_autoclose int fd = avb_server_make_socket(server, AVB_MSRP_ETH, msrp_mac);
|
||||
if (fd < 0) {
|
||||
errno = -fd;
|
||||
return NULL;
|
||||
|
|
@ -642,27 +643,26 @@ struct avb_msrp *avb_msrp_register(struct server *server)
|
|||
msrp = calloc(1, sizeof(*msrp));
|
||||
if (msrp == NULL) {
|
||||
res = -errno;
|
||||
goto error_close;
|
||||
goto error;
|
||||
}
|
||||
|
||||
msrp->server = server;
|
||||
spa_list_init(&msrp->attributes);
|
||||
|
||||
msrp->source = pw_loop_add_io(server->impl->loop, fd, SPA_IO_IN, true, on_socket_data, msrp);
|
||||
msrp->source = pw_loop_add_io(server->impl->loop, spa_steal_fd(fd), SPA_IO_IN, true, on_socket_data, msrp);
|
||||
if (msrp->source == NULL) {
|
||||
res = -errno;
|
||||
pw_log_error("msrp %p: can't create msrp source: %m", msrp);
|
||||
goto error_no_source;
|
||||
goto error_free;
|
||||
}
|
||||
avdecc_server_add_listener(server, &msrp->server_listener, &server_events, msrp);
|
||||
avb_mrp_add_listener(server->mrp, &msrp->mrp_listener, &mrp_events, msrp);
|
||||
|
||||
return (struct avb_msrp*)msrp;
|
||||
|
||||
error_no_source:
|
||||
error_free:
|
||||
free(msrp);
|
||||
error_close:
|
||||
close(fd);
|
||||
error:
|
||||
errno = -res;
|
||||
return NULL;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -6,6 +6,8 @@
|
|||
|
||||
#include <unistd.h>
|
||||
|
||||
#include <spa/utils/cleanup.h>
|
||||
|
||||
#include <pipewire/pipewire.h>
|
||||
|
||||
#include "mvrp.h"
|
||||
|
|
@ -255,9 +257,9 @@ static const struct avb_mrp_events mrp_events = {
|
|||
struct avb_mvrp *avb_mvrp_register(struct server *server)
|
||||
{
|
||||
struct mvrp *mvrp;
|
||||
int fd, res;
|
||||
int res;
|
||||
|
||||
fd = avb_server_make_socket(server, AVB_MVRP_ETH, mvrp_mac);
|
||||
spa_autoclose int fd = avb_server_make_socket(server, AVB_MVRP_ETH, mvrp_mac);
|
||||
if (fd < 0) {
|
||||
errno = -fd;
|
||||
return NULL;
|
||||
|
|
@ -265,27 +267,26 @@ struct avb_mvrp *avb_mvrp_register(struct server *server)
|
|||
mvrp = calloc(1, sizeof(*mvrp));
|
||||
if (mvrp == NULL) {
|
||||
res = -errno;
|
||||
goto error_close;
|
||||
goto error;
|
||||
}
|
||||
|
||||
mvrp->server = server;
|
||||
spa_list_init(&mvrp->attributes);
|
||||
|
||||
mvrp->source = pw_loop_add_io(server->impl->loop, fd, SPA_IO_IN, true, on_socket_data, mvrp);
|
||||
mvrp->source = pw_loop_add_io(server->impl->loop, spa_steal_fd(fd), SPA_IO_IN, true, on_socket_data, mvrp);
|
||||
if (mvrp->source == NULL) {
|
||||
res = -errno;
|
||||
pw_log_error("mvrp %p: can't create mvrp source: %m", mvrp);
|
||||
goto error_no_source;
|
||||
goto error_free;
|
||||
}
|
||||
avdecc_server_add_listener(server, &mvrp->server_listener, &server_events, mvrp);
|
||||
avb_mrp_add_listener(server->mrp, &mvrp->mrp_listener, &mrp_events, mvrp);
|
||||
|
||||
return (struct avb_mvrp*)mvrp;
|
||||
|
||||
error_no_source:
|
||||
error_free:
|
||||
free(mvrp);
|
||||
error_close:
|
||||
close(fd);
|
||||
error:
|
||||
errno = -res;
|
||||
return NULL;
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue