From 401df6e1996999e86db439d67b2556581d90decd Mon Sep 17 00:00:00 2001 From: Dmitry Sharshakov Date: Sat, 20 Jan 2024 10:26:25 +0300 Subject: [PATCH] module-rtp-sap: review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Barnabás Pőcze --- src/modules/module-rtp-sap.c | 21 +++++----- src/modules/module-rtp/ptp.h | 74 ++++++++++++++++++------------------ 2 files changed, 46 insertions(+), 49 deletions(-) diff --git a/src/modules/module-rtp-sap.c b/src/modules/module-rtp-sap.c index a875911ec..236eb9dcb 100644 --- a/src/modules/module-rtp-sap.c +++ b/src/modules/module-rtp-sap.c @@ -391,7 +391,7 @@ static int make_unix_socket(char *path, char *client_path) { fd = socket(AF_UNIX, SOCK_DGRAM, 0); if (fd == -1) { pw_log_warn("Failed to create PTP management socket"); - return 0; + return -1; } spa_zero(client_addr); @@ -400,7 +400,7 @@ static int make_unix_socket(char *path, char *client_path) { if (bind(fd, (struct sockaddr *)&client_addr, sizeof(client_addr)) == -1) { pw_log_warn("Failed to bind PTP management socket"); - return 0; + return -1; } spa_zero(server_addr); @@ -409,7 +409,7 @@ static int make_unix_socket(char *path, char *client_path) { if (connect(fd, (struct sockaddr*)&server_addr, sizeof(server_addr)) == -1) { pw_log_warn("Failed to connect PTP management socket"); - return 0; + return -1; } return fd; @@ -540,17 +540,14 @@ static int get_ip(const struct sockaddr_storage *sa, char *ip, size_t len, bool static void update_ts_refclk(struct impl *impl) { - if (!impl->ptp_mgmt_socket || !impl->ptp_fd) + if (!impl->ptp_mgmt_socket || impl->ptp_fd < 0) return; // Read if something is left in the socket int avail; ioctl(impl->ptp_fd, FIONREAD, &avail); - if (avail) { - void *tmp = malloc(avail); - read(impl->ptp_fd, tmp, avail); - free(tmp); - } + uint8_t tmp; + while (avail--) read(impl->ptp_fd, &tmp, 1); struct ptp_management_msg req; spa_zero(req); @@ -579,7 +576,7 @@ static void update_ts_refclk(struct impl *impl) uint8_t buf[sizeof(struct ptp_management_msg) + sizeof(struct ptp_parent_data_set)]; if (read(impl->ptp_fd, &buf, sizeof(buf)) == -1) { - pw_log_warn("Failed to send PTP management request: %m"); + pw_log_warn("Failed to receive PTP management response: %m"); return; } @@ -589,7 +586,7 @@ static void update_ts_refclk(struct impl *impl) uint16_t data_len = be16toh(res.management_message_length_be) - 2; if (data_len != sizeof(struct ptp_parent_data_set)) - pw_log_warn("Unexpected PTP GET PARENT_DATA_SET response length %u, expected 32", data_len); + pw_log_warn("Unexpected PTP GET PARENT_DATA_SET response length %u, expected %zu", data_len, sizeof(struct ptp_parent_data_set)); uint8_t *cid = res.clock_identity; if (memcmp(cid, impl->clock_id, 8) != 0) @@ -1703,7 +1700,7 @@ int pipewire__module_init(struct pw_impl_module *module, const char *args) if (!client_dir) client_dir = getenv("XDG_RUNTIME_DIR"); if (!client_dir) client_dir = "/tmp"; - snprintf(impl->ptp_client_path, 63, "%s/pipewire-ptp-mgmt.%d", client_dir, getpid()); + snprintf(impl->ptp_client_path, sizeof(impl->ptp_client_path), "%s/pipewire-ptp-mgmt.%d", client_dir, getpid()); // TODO: support UDP management access as well impl->ptp_fd = make_unix_socket(impl->ptp_mgmt_socket, impl->ptp_client_path); diff --git a/src/modules/module-rtp/ptp.h b/src/modules/module-rtp/ptp.h index 64a5931a7..3c874710b 100644 --- a/src/modules/module-rtp/ptp.h +++ b/src/modules/module-rtp/ptp.h @@ -12,46 +12,46 @@ extern "C" { #endif struct ptp_management_msg { - // 4 for major_sdo, 4 for msg_type - uint8_t major_sdo_id_message_type; - // 4 for minor, 4 for major - uint8_t ver; - uint16_t message_length_be; - uint8_t domain_number; - uint8_t minor_sdo_id; - uint16_t flags_be; - uint8_t correction_field[8]; - uint32_t message_type_specific; - uint8_t clock_identity[8]; - uint16_t source_port_id_be; - uint16_t sequence_id_be; - uint8_t control_field; - uint8_t log_message_interval; - uint8_t target_port_identity[8]; - uint16_t target_port_id_be; - uint8_t starting_boundary_hops; - uint8_t boundary_hops; - uint8_t action; - uint8_t reserved; - uint16_t tlv_type_be; - // length of data after this + 2 for management_id - uint16_t management_message_length_be; - uint16_t management_id_be; + // 4 for major_sdo, 4 for msg_type + uint8_t major_sdo_id_message_type; + // 4 for minor, 4 for major + uint8_t ver; + uint16_t message_length_be; + uint8_t domain_number; + uint8_t minor_sdo_id; + uint16_t flags_be; + uint8_t correction_field[8]; + uint32_t message_type_specific; + uint8_t clock_identity[8]; + uint16_t source_port_id_be; + uint16_t sequence_id_be; + uint8_t control_field; + uint8_t log_message_interval; + uint8_t target_port_identity[8]; + uint16_t target_port_id_be; + uint8_t starting_boundary_hops; + uint8_t boundary_hops; + uint8_t action; + uint8_t reserved; + uint16_t tlv_type_be; + // length of data after this + 2 for management_id + uint16_t management_message_length_be; + uint16_t management_id_be; } __attribute__((packed)); struct ptp_parent_data_set { - uint8_t parent_clock_id[8]; - uint16_t parent_port_id_be; - uint8_t parent_stats; - uint8_t reserved; - uint16_t log_variance_be; - int32_t phase_change_rate_be; - uint8_t gm_prio1; - uint8_t gm_clock_class; - uint8_t gm_clock_accuracy; - uint16_t gm_clock_variance_be; - uint8_t gm_prio2; - uint8_t gm_clock_id[8]; + uint8_t parent_clock_id[8]; + uint16_t parent_port_id_be; + uint8_t parent_stats; + uint8_t reserved; + uint16_t log_variance_be; + int32_t phase_change_rate_be; + uint8_t gm_prio1; + uint8_t gm_clock_class; + uint8_t gm_clock_accuracy; + uint16_t gm_clock_variance_be; + uint8_t gm_prio2; + uint8_t gm_clock_id[8]; } __attribute__((packed)); #ifdef __cplusplus