From fe489250ce102edf511e99669025934ec9587c63 Mon Sep 17 00:00:00 2001 From: =?utf8?q?J=C3=A9r=C3=A9mie=20Galarneau?= Date: Wed, 22 Jul 2020 16:15:44 -0400 Subject: [PATCH] payload: use fd_handle instead of raw file descriptors MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Using raw file descriptors with lttng_payloads introduces scary file descriptor corner-cases when mixing with asynchroneous communication and lttng_payloads. Since an lttng_payload doesn't own its file descriptors, attempting it is easy to fall into a situation where a file descriptor is referenced by an lttng_payload while the owner is destroyed. For instance, a userspace probe could be destroyed while its description is waiting to be sent to a client. The various use sites of the payload/payload_view APIs are adjusted. Utilities to send/recv fds through unix sockets using the payload and payload view interfaces are added as part of this commit as they use the payload's fd_handles. Signed-off-by: Jérémie Galarneau Change-Id: I44073f1b683af6c475e8e93c25a76910acf955e4 --- include/lttng/userspace-probe-internal.h | 15 +- include/lttng/userspace-probe.h | 4 +- src/bin/lttng-sessiond/client.c | 62 ++++++-- src/common/fd-handle.c | 10 +- src/common/payload-view.c | 44 +++--- src/common/payload-view.h | 41 ++--- src/common/payload.c | 68 +++++---- src/common/payload.h | 22 ++- src/common/unix.c | 169 +++++++++++++++++++++ src/common/unix.h | 15 ++ src/common/userspace-probe.c | 181 ++++++++++------------- src/lib/lttng-ctl/lttng-ctl.c | 44 +++--- tests/unit/test_payload.c | 89 ++++++++--- 13 files changed, 523 insertions(+), 241 deletions(-) diff --git a/include/lttng/userspace-probe-internal.h b/include/lttng/userspace-probe-internal.h index d9e5de254..ffe3f1cf9 100644 --- a/include/lttng/userspace-probe-internal.h +++ b/include/lttng/userspace-probe-internal.h @@ -11,6 +11,7 @@ #include #include +#include #include struct lttng_payload; @@ -97,9 +98,8 @@ struct lttng_userspace_probe_location_function { * binary_fd is a file descriptor to the executable file. It's open * early on to keep the backing inode valid over the course of the * intrumentation and use. It prevents deletion and reuse races. - * Set to -1 if not open. */ - int binary_fd; + struct fd_handle *binary_fd; enum lttng_userspace_probe_location_function_instrumentation_type instrumentation_type; }; @@ -112,9 +112,8 @@ struct lttng_userspace_probe_location_tracepoint { * binary_fd is a file descriptor to the executable file. It's open * early on to keep the backing inode valid over the course of the * intrumentation and use. It prevents deletion and reuse races. - * Set to -1 if not open. */ - int binary_fd; + struct fd_handle *binary_fd; }; LTTNG_HIDDEN @@ -127,14 +126,6 @@ int lttng_userspace_probe_location_create_from_payload( struct lttng_payload_view *view, struct lttng_userspace_probe_location **probe_location); -LTTNG_HIDDEN -int lttng_userspace_probe_location_function_set_binary_fd( - struct lttng_userspace_probe_location *location, int binary_fd); - -LTTNG_HIDDEN -int lttng_userspace_probe_location_tracepoint_set_binary_fd( - struct lttng_userspace_probe_location *location, int binary_fd); - /* * Returns a version of the location that is serialized to a contiguous region * of memory. Pass NULL to buffer to only get the storage requirement of the diff --git a/include/lttng/userspace-probe.h b/include/lttng/userspace-probe.h index 7215bd967..5a6fa51a0 100644 --- a/include/lttng/userspace-probe.h +++ b/include/lttng/userspace-probe.h @@ -120,7 +120,7 @@ extern const char *lttng_userspace_probe_location_function_get_function_name( /* * Get the FD to the target binary file to the probe location of the function - * type. + * type. The FD is only valid for the duration of the lifetime of `location`. */ extern int lttng_userspace_probe_location_function_get_binary_fd( const struct lttng_userspace_probe_location *location); @@ -191,7 +191,7 @@ extern const char *lttng_userspace_probe_location_tracepoint_get_provider_name( /* * Get the FD to the target binary file to the probe location of the tracepoint - * type. + * type. The FD is only valid for the duration of the lifetime of `location`. */ extern int lttng_userspace_probe_location_tracepoint_get_binary_fd( const struct lttng_userspace_probe_location *location); diff --git a/src/bin/lttng-sessiond/client.c b/src/bin/lttng-sessiond/client.c index 74bbbe9b2..1bf55f60b 100644 --- a/src/bin/lttng-sessiond/client.c +++ b/src/bin/lttng-sessiond/client.c @@ -13,6 +13,7 @@ #include "common/dynamic-array.h" #include "common/payload.h" #include "common/payload-view.h" +#include "common/fd-handle.h" #include "common/sessiond-comm/sessiond-comm.h" #include "common/payload.h" #include "common/payload-view.h" @@ -92,7 +93,7 @@ static int setup_lttng_msg(struct command_ctx *cmd_ctx, }; lttng_dynamic_buffer_set_size(&cmd_ctx->reply_payload.buffer, 0); - lttng_dynamic_array_clear(&cmd_ctx->reply_payload._fds); + lttng_dynamic_pointer_array_clear(&cmd_ctx->reply_payload._fd_handles); cmd_ctx->lttng_msg_size = total_msg_size; @@ -592,9 +593,10 @@ static unsigned int lttng_sessions_count(uid_t uid, gid_t gid) static int receive_userspace_probe(struct command_ctx *cmd_ctx, int sock, int *sock_error, struct lttng_event *event) { - int fd, ret; + int fd = -1, ret; struct lttng_userspace_probe_location *probe_location; struct lttng_payload probe_location_payload; + struct fd_handle *handle = NULL; /* * Create a payload to store the serialized version of the probe @@ -633,13 +635,25 @@ static int receive_userspace_probe(struct command_ctx *cmd_ctx, int sock, goto error; } - ret = lttng_payload_push_fd(&probe_location_payload, fd); + handle = fd_handle_create(fd); + if (!handle) { + ret = LTTNG_ERR_NOMEM; + goto error; + } + + /* Transferred to the handle. */ + fd = -1; + + ret = lttng_payload_push_fd_handle(&probe_location_payload, handle); if (ret) { ERR("Failed to add userspace probe file descriptor to payload"); ret = LTTNG_ERR_NOMEM; goto error; } + fd_handle_put(handle); + handle = NULL; + { struct lttng_payload_view view = lttng_payload_view_from_payload( &probe_location_payload, 0, -1); @@ -662,6 +676,13 @@ static int receive_userspace_probe(struct command_ctx *cmd_ctx, int sock, } error: + if (fd >= 0) { + if (close(fd)) { + PERROR("Failed to close userspace probe location binary fd"); + } + } + + fd_handle_put(handle); lttng_payload_reset(&probe_location_payload); return ret; } @@ -699,6 +720,7 @@ static int check_rotate_compatible(void) static int send_unix_sock(int sock, struct lttng_payload_view *view) { int ret; + const int fd_count = lttng_payload_view_get_fd_handle_count(view); /* Check valid length */ if (view->buffer.size == 0) { @@ -712,10 +734,33 @@ static int send_unix_sock(int sock, struct lttng_payload_view *view) goto end; } - if (lttng_dynamic_array_get_count(&view->_fds) > 0) { + if (fd_count > 0) { + int i; + struct lttng_dynamic_array raw_fds; + + /* + * Never holds ownership of the FDs; this is just used + * to put the FDs in a contiguous array. + */ + lttng_dynamic_array_init(&raw_fds, sizeof(int), NULL); + + for (i = 0; i < fd_count; i++) { + struct fd_handle *handle = + lttng_payload_view_pop_fd_handle(view); + const int raw_fd = fd_handle_get_fd(handle); + + ret = lttng_dynamic_array_add_element(&raw_fds, &raw_fd); + fd_handle_put(handle); + if (ret) { + lttng_dynamic_array_reset(&raw_fds); + goto end; + } + } + ret = lttcomm_send_fds_unix_sock(sock, - (const int *) view->_fds.buffer.data, - lttng_dynamic_array_get_count(&view->_fds)); + (const int *) raw_fds.buffer.data, + fd_count); + lttng_dynamic_array_reset(&raw_fds); } end: @@ -2266,8 +2311,7 @@ static void *thread_manage_clients(void *data) .gid = UINT32_MAX, }; cmd_ctx.session = NULL; - lttng_dynamic_buffer_set_size(&cmd_ctx.reply_payload.buffer, 0); - lttng_dynamic_array_clear(&cmd_ctx.reply_payload._fds); + lttng_payload_clear(&cmd_ctx.reply_payload); cmd_ctx.lttng_msg_size = 0; DBG("Accepting client command ..."); @@ -2410,7 +2454,7 @@ static void *thread_manage_clients(void *data) sizeof(llm)); assert(cmd_ctx.lttng_msg_size == cmd_ctx.reply_payload.buffer.size); - llm->fd_count = lttng_payload_view_get_fd_count(&view); + llm->fd_count = lttng_payload_view_get_fd_handle_count(&view); DBG("Sending response (size: %d, retcode: %s (%d))", cmd_ctx.lttng_msg_size, diff --git a/src/common/fd-handle.c b/src/common/fd-handle.c index a9c7be4a9..ddf70afad 100644 --- a/src/common/fd-handle.c +++ b/src/common/fd-handle.c @@ -58,14 +58,20 @@ end: LTTNG_HIDDEN void fd_handle_get(struct fd_handle *handle) { - assert(handle); + if (!handle) { + return; + } + urcu_ref_get(&handle->ref); } LTTNG_HIDDEN void fd_handle_put(struct fd_handle *handle) { - assert(handle); + if (!handle) { + return; + } + urcu_ref_put(&handle->ref, fd_handle_release); } diff --git a/src/common/payload-view.c b/src/common/payload-view.c index 8c99c56ef..43ddf3132 100644 --- a/src/common/payload-view.c +++ b/src/common/payload-view.c @@ -19,7 +19,7 @@ struct lttng_payload_view lttng_payload_view_from_payload( return (struct lttng_payload_view) { .buffer = lttng_buffer_view_from_dynamic_buffer( &payload->buffer, offset, len), - ._fds = payload->_fds, + ._fd_handles = payload->_fd_handles, }; } @@ -31,9 +31,9 @@ struct lttng_payload_view lttng_payload_view_from_view( return (struct lttng_payload_view) { .buffer = lttng_buffer_view_from_view( &view->buffer, offset, len), - ._fds = view->_fds, - ._iterator.p_fds_position = view->_iterator.p_fds_position ?: - &view->_iterator.fds_position, + ._fd_handles = view->_fd_handles, + ._iterator.p_fd_handles_position = view->_iterator.p_fd_handles_position ?: + &view->_iterator.fd_handles_position, }; } @@ -70,7 +70,7 @@ struct lttng_payload_view lttng_payload_view_init_from_buffer( } LTTNG_HIDDEN -int lttng_payload_view_get_fd_count(struct lttng_payload_view *payload_view) +int lttng_payload_view_get_fd_handle_count(struct lttng_payload_view *payload_view) { int ret; size_t position; @@ -80,43 +80,43 @@ int lttng_payload_view_get_fd_count(struct lttng_payload_view *payload_view) goto end; } - ret = lttng_dynamic_array_get_count(&payload_view->_fds); + ret = lttng_dynamic_pointer_array_get_count(&payload_view->_fd_handles); if (ret < 0) { goto end; } - position = payload_view->_iterator.p_fds_position ? - *payload_view->_iterator.p_fds_position : - payload_view->_iterator.fds_position; + position = payload_view->_iterator.p_fd_handles_position ? + *payload_view->_iterator.p_fd_handles_position : + payload_view->_iterator.fd_handles_position; ret = ret - (int) position; end: return ret; } LTTNG_HIDDEN -int lttng_payload_view_pop_fd(struct lttng_payload_view *view) +struct fd_handle *lttng_payload_view_pop_fd_handle( + struct lttng_payload_view *view) { - int ret = 0; - size_t fd_count; + struct fd_handle *handle = NULL; + size_t fd_handle_count; size_t *pos; if (!view) { - ret = -1; goto end; } - fd_count = lttng_dynamic_array_get_count(&view->_fds); - pos = view->_iterator.p_fds_position ? view->_iterator.p_fds_position : - &view->_iterator.fds_position; - - if (*pos >= fd_count) { - ret = -1; + fd_handle_count = lttng_payload_view_get_fd_handle_count(view); + if (fd_handle_count == 0) { goto end; } - ret = *((int *) lttng_dynamic_array_get_element( - &view->_fds, *pos)); + pos = view->_iterator.p_fd_handles_position ? + view->_iterator.p_fd_handles_position : + &view->_iterator.fd_handles_position; + handle = lttng_dynamic_pointer_array_get_pointer(&view->_fd_handles, + *pos); (*pos)++; + fd_handle_get(handle); end: - return ret; + return handle; } diff --git a/src/common/payload-view.h b/src/common/payload-view.h index 9e385842b..a07c0be00 100644 --- a/src/common/payload-view.h +++ b/src/common/payload-view.h @@ -12,6 +12,7 @@ #include struct lttng_payload; +struct fd_handle; /* * An lttng_payload_view references a payload and allows code to share @@ -21,22 +22,23 @@ struct lttng_payload; * payload view) is modified. * * While a payload view does not allow users to modify the underlying bytes - * of the payload, it can be used to 'pop' file descriptors using an iterator - * belonging to the top-level payload view. + * of the payload, it can be used to 'pop' file descriptor handles using an + * iterator belonging to the top-level payload view. * * Hence, a payload view created from a payload or a dynamic buffer contains - * an implicit file descriptor iterator. Any payload view created from another - * payload view will share the same underlying file descriptor iterator. + * an implicit file descriptor handle iterator. Any payload view created from + * another payload view will share the same underlying file descriptor handle + * iterator. * - * The rationale for this is that a payload is never consumed directly, it - * must be consumed through a payload view. + * The rationale for this is that a payload is never consumed directly, it must + * be consumed through a payload view. * * Typically, a payload view will be used to rebuild a previously serialized * object hierarchy. Sharing an underlying iterator allows aggregate objects * to provide a restricted view of the payload to their members, which will - * report the number of bytes consumed and `pop` the file descriptors they + * report the number of bytes consumed and `pop` the file descriptor handle they * should own. In return, those objects can create an even narrower view for - * their children, allowing them to also consume file descriptors. + * their children, allowing them to also consume file descriptor handles. * * Note that a payload view never assumes any ownership of the underlying * payload. @@ -44,10 +46,10 @@ struct lttng_payload; struct lttng_payload_view { struct lttng_buffer_view buffer; /* private */ - const struct lttng_dynamic_array _fds; + const struct lttng_dynamic_pointer_array _fd_handles; struct { - size_t *p_fds_position; - size_t fds_position; + size_t *p_fd_handles_position; + size_t fd_handles_position; } _iterator; }; @@ -130,24 +132,27 @@ struct lttng_payload_view lttng_payload_view_init_from_buffer( const char *src, size_t offset, ptrdiff_t len); /** - * Get the number of file descriptors left in a payload view. + * Get the number of file descriptor handles left in a payload view. * * @payload Payload instance * - * Returns the number of file descriptors left on success, -1 on error. + * Returns the number of file descriptor handles left on success, -1 on error. */ LTTNG_HIDDEN -int lttng_payload_view_get_fd_count(struct lttng_payload_view *payload_view); +int lttng_payload_view_get_fd_handle_count( + struct lttng_payload_view *payload_view); /** - * Pop an fd from a payload view. - * No ownership of the file descriptor is assumed by the payload. + * Pop an fd handle from a payload view. + * + * A reference to the returned fd_handle is acquired on behalf of the caller. * * @payload Payload instance * - * Returns a file descriptor on success, -1 on error. + * Returns an fd_handle on success, -1 on error. */ LTTNG_HIDDEN -int lttng_payload_view_pop_fd(struct lttng_payload_view *payload_view); +struct fd_handle *lttng_payload_view_pop_fd_handle( + struct lttng_payload_view *payload_view); #endif /* LTTNG_PAYLOAD_VIEW_H */ diff --git a/src/common/payload.c b/src/common/payload.c index c4faece18..70b2c7e5a 100644 --- a/src/common/payload.c +++ b/src/common/payload.c @@ -10,12 +10,21 @@ #include #include +static +void release_fd_handle_ref(void *ptr) +{ + struct fd_handle *fd_handle = ptr; + + fd_handle_put(fd_handle); +} + LTTNG_HIDDEN void lttng_payload_init(struct lttng_payload *payload) { assert(payload); lttng_dynamic_buffer_init(&payload->buffer); - lttng_dynamic_array_init(&payload->_fds, sizeof(int), NULL); + lttng_dynamic_pointer_array_init(&payload->_fd_handles, + release_fd_handle_ref); } LTTNG_HIDDEN @@ -28,38 +37,33 @@ int lttng_payload_copy(const struct lttng_payload *src_payload, ret = lttng_dynamic_buffer_append_buffer( &dst_payload->buffer, &src_payload->buffer); if (ret) { - goto error; + goto end; } - for (i = 0; i < lttng_dynamic_array_get_count(&src_payload->_fds); + for (i = 0; i < lttng_dynamic_pointer_array_get_count( + &src_payload->_fd_handles); i++) { - int dst_fd; - const int src_fd = *((int *) lttng_dynamic_array_get_element( - &src_payload->_fds, i)); - - dst_fd = dup(src_fd); - if (dst_fd < 0) { - PERROR("Failed to duplicate file descriptor while copying a payload"); - ret = dst_fd; - goto error; + struct fd_handle *new_fd_handle; + const struct fd_handle *src_fd_handle = + lttng_dynamic_pointer_array_get_pointer( + &src_payload->_fd_handles, i); + + new_fd_handle = fd_handle_copy(src_fd_handle); + if (!new_fd_handle) { + PERROR("Failed to copy fd_handle while copying a payload"); + ret = -1; + goto end; } - ret = lttng_payload_push_fd(dst_payload, dst_fd); + ret = lttng_payload_push_fd_handle(dst_payload, new_fd_handle); + fd_handle_put(new_fd_handle); if (ret) { - const int close_ret = close(dst_fd); - - if (close_ret < 0) { - PERROR("Failed to close duplicated file descriptor while copying a payload"); - } - - goto error; + goto end; } } end: return ret; -error: - goto end; } LTTNG_HIDDEN @@ -70,11 +74,19 @@ void lttng_payload_reset(struct lttng_payload *payload) } lttng_dynamic_buffer_reset(&payload->buffer); - lttng_dynamic_array_reset(&payload->_fds); + lttng_dynamic_pointer_array_reset(&payload->_fd_handles); } LTTNG_HIDDEN -int lttng_payload_push_fd(struct lttng_payload *payload, int fd) +void lttng_payload_clear(struct lttng_payload *payload) +{ + lttng_dynamic_buffer_set_size(&payload->buffer, 0); + lttng_dynamic_pointer_array_clear(&payload->_fd_handles); +} + +LTTNG_HIDDEN +int lttng_payload_push_fd_handle(struct lttng_payload *payload, + struct fd_handle *fd_handle) { int ret; @@ -83,7 +95,13 @@ int lttng_payload_push_fd(struct lttng_payload *payload, int fd) goto end; } - ret = lttng_dynamic_array_add_element(&payload->_fds, &fd); + ret = lttng_dynamic_pointer_array_add_pointer( + &payload->_fd_handles, fd_handle); + if (ret) { + goto end; + } + + fd_handle_get(fd_handle); end: return ret; } diff --git a/src/common/payload.h b/src/common/payload.h index 465a097cf..a8f66f36b 100644 --- a/src/common/payload.h +++ b/src/common/payload.h @@ -10,6 +10,7 @@ #include #include +#include /* * An lttng_payload encompasses the 'data' (bytes) and any passed file @@ -19,7 +20,7 @@ struct lttng_payload { struct lttng_dynamic_buffer buffer; /* private */ - struct lttng_dynamic_array _fds; + struct lttng_dynamic_pointer_array _fd_handles; }; /* @@ -34,20 +35,31 @@ LTTNG_HIDDEN int lttng_payload_copy(const struct lttng_payload *src_payload, struct lttng_payload *dst_payload); -/* Release any memory used by the payload. */ +/* Release any memory and references held by the payload. */ LTTNG_HIDDEN void lttng_payload_reset(struct lttng_payload *payload); +/* + * Empty the contents of a payload, releasing all references held. + * This should be used to put a payload in a re-usable state. + * + * lttng_payload_reset must still be called on an lttng_payload to + * free all allocated memory. + */ +LTTNG_HIDDEN +void lttng_payload_clear(struct lttng_payload *payload); + /** * Add an fd to the payload. - * No ownership of the file descriptor is assumed by the payload. + * The payload acquires a reference to the fd_handle. * * @payload Payload instance - * @fd File descriptor to add to the payload + * @fd_handle File descriptor handle to add to the payload * * Returns 0 on success, -1 on allocation error. */ LTTNG_HIDDEN -int lttng_payload_push_fd(struct lttng_payload *payload, int fd); +int lttng_payload_push_fd_handle(struct lttng_payload *payload, + struct fd_handle *fd_handle); #endif /* LTTNG_PAYLOAD_H */ diff --git a/src/common/unix.c b/src/common/unix.c index f50611b5b..502002425 100644 --- a/src/common/unix.c +++ b/src/common/unix.c @@ -19,6 +19,7 @@ #include #include +#include #include "unix.h" @@ -435,6 +436,73 @@ ssize_t lttcomm_send_fds_unix_sock(int sock, const int *fds, size_t nb_fd) return ret; } +/* + * Send the fd(s) of a payload view over a unix socket. + * + * Returns the size of data sent, or negative error value. + */ +static +ssize_t _lttcomm_send_payload_view_fds_unix_sock(int sock, + struct lttng_payload_view *view, + bool blocking) +{ + int i; + ssize_t ret; + struct lttng_dynamic_array raw_fds; + const int fd_count = lttng_payload_view_get_fd_handle_count(view); + + lttng_dynamic_array_init(&raw_fds, sizeof(int), NULL); + + /* + * Prepare a contiguous array of file descriptors to send them. + * + * Note that the reference to each fd is released during the iteration; + * we're just getting the numerical value of the fds to conform to the + * syscall's interface. We rely on the fact that "view" must remain + * valid for the duration of the call and that the underlying payload + * owns a reference to the fd_handles. + */ + for (i = 0; i < fd_count; i++) { + struct fd_handle *handle = + lttng_payload_view_pop_fd_handle(view); + const int raw_fd = fd_handle_get_fd(handle); + const int add_ret = lttng_dynamic_array_add_element( + &raw_fds, &raw_fd); + + fd_handle_put(handle); + if (add_ret) { + ret = -LTTNG_ERR_NOMEM; + goto end; + } + } + + if (blocking) { + ret = lttcomm_send_fds_unix_sock(sock, + (const int *) raw_fds.buffer.data, fd_count); + } else { + ret = lttcomm_send_fds_unix_sock_non_block(sock, + (const int *) raw_fds.buffer.data, fd_count); + } + +end: + lttng_dynamic_array_reset(&raw_fds); + return ret; +} + +LTTNG_HIDDEN +ssize_t lttcomm_send_payload_view_fds_unix_sock(int sock, + struct lttng_payload_view *view) +{ + return _lttcomm_send_payload_view_fds_unix_sock(sock, view, true); +} + +LTTNG_HIDDEN +ssize_t lttcomm_send_payload_view_fds_unix_sock_non_block(int sock, + struct lttng_payload_view *view) +{ + return _lttcomm_send_payload_view_fds_unix_sock(sock, view, false); +} + /* * Send a message accompanied by fd(s) over a unix socket. * Only use for non blocking socket. @@ -629,6 +697,107 @@ end: return ret; } +static +void close_raw_fd(void *ptr) +{ + const int raw_fd = *((const int *) ptr); + + if (raw_fd >= 0) { + const int ret = close(raw_fd); + + if (ret) { + PERROR("Failed to close file descriptor %d", raw_fd); + } + } +} + +static +enum lttng_error_code add_fds_to_payload(struct lttng_dynamic_array *raw_fds, + struct lttng_payload *payload) +{ + int i; + enum lttng_error_code ret_code = LTTNG_OK; + const int fd_count = lttng_dynamic_array_get_count(raw_fds); + + for (i = 0; i < fd_count; i++) { + int ret; + struct fd_handle *handle; + int *raw_fd = (int *) lttng_dynamic_array_get_element( + raw_fds, i); + + handle = fd_handle_create(*raw_fd); + if (!handle) { + ret_code = LTTNG_ERR_NOMEM; + goto end; + } + + /* FD ownership transferred to the handle. */ + *raw_fd = -1; + + ret = lttng_payload_push_fd_handle(payload, handle); + fd_handle_put(handle); + if (ret) { + ret_code = LTTNG_ERR_NOMEM; + goto end; + } + } + +end: + return ret_code; +} + +static +ssize_t _lttcomm_recv_payload_fds_unix_sock(int sock, size_t nb_fd, + struct lttng_payload *payload, bool blocking) +{ + enum lttng_error_code add_ret; + ssize_t ret; + struct lttng_dynamic_array raw_fds; + + lttng_dynamic_array_init(&raw_fds, sizeof(int), close_raw_fd); + ret = lttng_dynamic_array_set_count(&raw_fds, nb_fd); + if (ret) { + ret = -LTTNG_ERR_NOMEM; + goto end; + } + + if (blocking) { + ret = lttcomm_recv_fds_unix_sock( + sock, (int *) raw_fds.buffer.data, nb_fd); + } else { + ret = lttcomm_recv_fds_unix_sock_non_block( + sock, (int *) raw_fds.buffer.data, nb_fd); + } + + if (ret < 0) { + goto end; + } + + add_ret = add_fds_to_payload(&raw_fds, payload); + if (add_ret != LTTNG_OK) { + ret = - (int) add_ret; + goto end; + } + +end: + lttng_dynamic_array_reset(&raw_fds); + return ret; +} + +LTTNG_HIDDEN +ssize_t lttcomm_recv_payload_fds_unix_sock(int sock, size_t nb_fd, + struct lttng_payload *payload) +{ + return _lttcomm_recv_payload_fds_unix_sock(sock, nb_fd, payload, true); +} + +LTTNG_HIDDEN +ssize_t lttcomm_recv_payload_fds_unix_sock_non_block(int sock, size_t nb_fd, + struct lttng_payload *payload) +{ + return _lttcomm_recv_payload_fds_unix_sock(sock, nb_fd, payload, false); +} + /* * Recv a message accompanied by fd(s) from a non-blocking unix socket. * Only use with non-blocking sockets. diff --git a/src/common/unix.h b/src/common/unix.h index a975543b3..4f69e6d48 100644 --- a/src/common/unix.h +++ b/src/common/unix.h @@ -13,6 +13,8 @@ #include #include +#include +#include LTTNG_HIDDEN int lttcomm_create_unix_sock(const char *pathname); @@ -31,13 +33,26 @@ int lttcomm_close_unix_sock(int sock); LTTNG_HIDDEN ssize_t lttcomm_send_fds_unix_sock(int sock, const int *fds, size_t nb_fd); LTTNG_HIDDEN +ssize_t lttcomm_send_payload_view_fds_unix_sock(int sock, + struct lttng_payload_view *view); +LTTNG_HIDDEN ssize_t lttcomm_send_fds_unix_sock_non_block( int sock, const int *fds, size_t nb_fd); +LTTNG_HIDDEN +ssize_t lttcomm_send_payload_view_fds_unix_sock_non_block(int sock, + struct lttng_payload_view *view); + /* Recv a message accompanied by fd(s) from a unix socket */ LTTNG_HIDDEN ssize_t lttcomm_recv_fds_unix_sock(int sock, int *fds, size_t nb_fd); LTTNG_HIDDEN +ssize_t lttcomm_recv_payload_fds_unix_sock(int sock, size_t nb_fd, + struct lttng_payload *payload); +LTTNG_HIDDEN ssize_t lttcomm_recv_fds_unix_sock_non_block(int sock, int *fds, size_t nb_fd); +LTTNG_HIDDEN +ssize_t lttcomm_recv_payload_fds_unix_sock_non_block(int sock, size_t nb_fd, + struct lttng_payload *payload); LTTNG_HIDDEN ssize_t lttcomm_recv_unix_sock(int sock, void *buf, size_t len); diff --git a/src/common/userspace-probe.c b/src/common/userspace-probe.c index 61ba309df..5e66f41c1 100644 --- a/src/common/userspace-probe.c +++ b/src/common/userspace-probe.c @@ -19,6 +19,16 @@ #include #include +static +int lttng_userspace_probe_location_function_set_binary_fd( + struct lttng_userspace_probe_location *location, + struct fd_handle *binary_fd); + +static +int lttng_userspace_probe_location_tracepoint_set_binary_fd( + struct lttng_userspace_probe_location *location, + struct fd_handle *binary_fd); + enum lttng_userspace_probe_location_lookup_method_type lttng_userspace_probe_location_lookup_method_get_type( const struct lttng_userspace_probe_location_lookup_method *lookup_method) @@ -95,11 +105,7 @@ void lttng_userspace_probe_location_function_destroy( free(location_function->function_name); free(location_function->binary_path); - if (location_function->binary_fd >= 0) { - if (close(location_function->binary_fd)) { - PERROR("close"); - } - } + fd_handle_put(location_function->binary_fd); free(location); } @@ -120,11 +126,7 @@ void lttng_userspace_probe_location_tracepoint_destroy( free(location_tracepoint->probe_name); free(location_tracepoint->provider_name); free(location_tracepoint->binary_path); - if (location_tracepoint->binary_fd >= 0) { - if (close(location_tracepoint->binary_fd)) { - PERROR("close"); - } - } + fd_handle_put(location_tracepoint->binary_fd); free(location); } @@ -225,7 +227,8 @@ static bool lttng_userspace_probe_location_function_is_equal( goto end; } - is_equal = fd_is_equal(a->binary_fd, b->binary_fd); + is_equal = fd_is_equal(a->binary_fd ? fd_handle_get_fd(a->binary_fd) : -1, + b->binary_fd ? fd_handle_get_fd(b->binary_fd) : -1); end: return is_equal; } @@ -237,6 +240,7 @@ lttng_userspace_probe_location_function_create_no_check(const char *binary_path, bool open_binary) { int binary_fd = -1; + struct fd_handle *binary_fd_handle = NULL; char *function_name_copy = NULL, *binary_path_copy = NULL; struct lttng_userspace_probe_location *ret = NULL; struct lttng_userspace_probe_location_function *location; @@ -247,7 +251,13 @@ lttng_userspace_probe_location_function_create_no_check(const char *binary_path, PERROR("Error opening the binary"); goto error; } - } else { + + binary_fd_handle = fd_handle_create(binary_fd); + if (!binary_fd) { + goto error; + } + + /* Ownership transferred to fd_handle. */ binary_fd = -1; } @@ -271,7 +281,8 @@ lttng_userspace_probe_location_function_create_no_check(const char *binary_path, location->function_name = function_name_copy; location->binary_path = binary_path_copy; - location->binary_fd = binary_fd; + location->binary_fd = binary_fd_handle; + binary_fd_handle = NULL; location->instrumentation_type = LTTNG_USERSPACE_PROBE_LOCATION_FUNCTION_INSTRUMENTATION_TYPE_ENTRY; @@ -289,6 +300,7 @@ error: PERROR("Error closing binary fd in error path"); } } + fd_handle_put(binary_fd_handle); end: return ret; } @@ -323,7 +335,8 @@ static bool lttng_userspace_probe_location_tracepoint_is_equal( goto end; } - is_equal = fd_is_equal(a->binary_fd, b->binary_fd); + is_equal = fd_is_equal(a->binary_fd ? fd_handle_get_fd(a->binary_fd) : -1, + b->binary_fd ? fd_handle_get_fd(b->binary_fd) : -1); end: return is_equal; @@ -336,6 +349,7 @@ lttng_userspace_probe_location_tracepoint_create_no_check(const char *binary_pat bool open_binary) { int binary_fd = -1; + struct fd_handle *binary_fd_handle = NULL; char *probe_name_copy = NULL; char *provider_name_copy = NULL; char *binary_path_copy = NULL; @@ -348,7 +362,13 @@ lttng_userspace_probe_location_tracepoint_create_no_check(const char *binary_pat PERROR("open"); goto error; } - } else { + + binary_fd_handle = fd_handle_create(binary_fd); + if (!binary_fd) { + goto error; + } + + /* Ownership transferred to fd_handle. */ binary_fd = -1; } @@ -379,7 +399,8 @@ lttng_userspace_probe_location_tracepoint_create_no_check(const char *binary_pat location->probe_name = probe_name_copy; location->provider_name = provider_name_copy; location->binary_path = binary_path_copy; - location->binary_fd = binary_fd; + location->binary_fd = binary_fd_handle; + binary_fd_handle = NULL; ret = &location->parent; ret->lookup_method = lookup_method; @@ -396,6 +417,7 @@ error: PERROR("Error closing binary fd in error path"); } } + fd_handle_put(binary_fd_handle); end: return ret; } @@ -519,10 +541,12 @@ lttng_userspace_probe_location_function_copy( struct lttng_userspace_probe_location_lookup_method *lookup_method = NULL; const char *binary_path = NULL; const char *function_name = NULL; - int fd, new_fd; + struct lttng_userspace_probe_location_function *function_location; assert(location); assert(location->type == LTTNG_USERSPACE_PROBE_LOCATION_TYPE_FUNCTION); + function_location = container_of( + location, typeof(*function_location), parent); /* Get probe location fields */ binary_path = lttng_userspace_probe_location_function_get_binary_path(location); @@ -537,19 +561,6 @@ lttng_userspace_probe_location_function_copy( goto error; } - /* Duplicate the binary fd */ - fd = lttng_userspace_probe_location_function_get_binary_fd(location); - if (fd == -1) { - ERR("Error getting file descriptor to binary"); - goto error; - } - - new_fd = dup(fd); - if (new_fd == -1) { - PERROR("Error duplicating file descriptor to binary"); - goto error; - } - /* * Duplicate probe location method fields */ @@ -561,12 +572,12 @@ lttng_userspace_probe_location_function_copy( lttng_userspace_probe_location_lookup_method_function_elf_copy( location->lookup_method); if (!lookup_method) { - goto close_fd; + goto error; } break; default: /* Invalid probe location lookup method. */ - goto close_fd; + goto error; } /* Create the probe_location */ @@ -577,7 +588,8 @@ lttng_userspace_probe_location_function_copy( } /* Set the duplicated fd to the new probe_location */ - if (lttng_userspace_probe_location_function_set_binary_fd(new_location, new_fd) < 0) { + if (lttng_userspace_probe_location_function_set_binary_fd(new_location, + function_location->binary_fd) < 0) { goto destroy_probe_location; } @@ -587,10 +599,6 @@ destroy_probe_location: lttng_userspace_probe_location_destroy(new_location); destroy_lookup_method: lttng_userspace_probe_location_lookup_method_destroy(lookup_method); -close_fd: - if (close(new_fd) < 0) { - PERROR("Error closing duplicated file descriptor in error path"); - } error: new_location = NULL; end: @@ -607,12 +615,14 @@ lttng_userspace_probe_location_tracepoint_copy( const char *binary_path = NULL; const char *probe_name = NULL; const char *provider_name = NULL; - int fd, new_fd; + struct lttng_userspace_probe_location_tracepoint *tracepoint_location; assert(location); assert(location->type == LTTNG_USERSPACE_PROBE_LOCATION_TYPE_TRACEPOINT); + tracepoint_location = container_of( + location, typeof(*tracepoint_location), parent); - /* Get probe location fields */ + /* Get probe location fields */ binary_path = lttng_userspace_probe_location_tracepoint_get_binary_path(location); if (!binary_path) { ERR("Userspace probe binary path is NULL"); @@ -631,19 +641,6 @@ lttng_userspace_probe_location_tracepoint_copy( goto error; } - /* Duplicate the binary fd */ - fd = lttng_userspace_probe_location_tracepoint_get_binary_fd(location); - if (fd == -1) { - ERR("Error getting file descriptor to binary"); - goto error; - } - - new_fd = dup(fd); - if (new_fd == -1) { - PERROR("Error duplicating file descriptor to binary"); - goto error; - } - /* * Duplicate probe location method fields */ @@ -655,12 +652,12 @@ lttng_userspace_probe_location_tracepoint_copy( lttng_userspace_probe_location_lookup_method_tracepoint_sdt_copy( location->lookup_method); if (!lookup_method) { - goto close_fd; + goto error; } break; default: /* Invalid probe location lookup method. */ - goto close_fd; + goto error; } /* Create the probe_location */ @@ -671,7 +668,8 @@ lttng_userspace_probe_location_tracepoint_copy( } /* Set the duplicated fd to the new probe_location */ - if (lttng_userspace_probe_location_tracepoint_set_binary_fd(new_location, new_fd) < 0) { + if (lttng_userspace_probe_location_tracepoint_set_binary_fd(new_location, + tracepoint_location->binary_fd) < 0) { goto destroy_probe_location; } @@ -681,10 +679,6 @@ destroy_probe_location: lttng_userspace_probe_location_destroy(new_location); destroy_lookup_method: lttng_userspace_probe_location_lookup_method_destroy(lookup_method); -close_fd: - if (close(new_fd) < 0) { - PERROR("Error closing duplicated file descriptor in error path"); - } error: new_location = NULL; end: @@ -802,7 +796,8 @@ int lttng_userspace_probe_location_function_get_binary_fd( function_location = container_of(location, struct lttng_userspace_probe_location_function, parent); - ret = function_location->binary_fd; + ret = function_location->binary_fd ? + fd_handle_get_fd(function_location->binary_fd) : -1; end: return ret; } @@ -867,7 +862,8 @@ int lttng_userspace_probe_location_tracepoint_get_binary_fd( tracepoint_location = container_of(location, struct lttng_userspace_probe_location_tracepoint, parent); - ret = tracepoint_location->binary_fd; + ret = tracepoint_location->binary_fd ? + fd_handle_get_fd(tracepoint_location->binary_fd) : -1; end: return ret; } @@ -1015,7 +1011,7 @@ int lttng_userspace_probe_location_function_serialize( ret = -LTTNG_ERR_INVALID; goto end; } - ret = lttng_payload_push_fd( + ret = lttng_payload_push_fd_handle( payload, location_function->binary_fd); if (ret) { ret = -LTTNG_ERR_INVALID; @@ -1109,7 +1105,7 @@ int lttng_userspace_probe_location_tracepoint_serialize( ret = -LTTNG_ERR_INVALID; goto end; } - ret = lttng_payload_push_fd( + ret = lttng_payload_push_fd_handle( payload, location_tracepoint->binary_fd); if (ret) { ret = -LTTNG_ERR_INVALID; @@ -1191,15 +1187,10 @@ int lttng_userspace_probe_location_function_create_from_payload( char *function_name = NULL, *binary_path = NULL; int ret = 0; size_t expected_size; - const int binary_fd = lttng_payload_view_pop_fd(view); + struct fd_handle *binary_fd = lttng_payload_view_pop_fd_handle(view); assert(location); - if (binary_fd < 0) { - ret = -LTTNG_ERR_INVALID; - goto end; - } - if (view->buffer.size < sizeof(*location_function_comm)) { ret = -LTTNG_ERR_INVALID; goto end; @@ -1254,17 +1245,13 @@ int lttng_userspace_probe_location_function_create_from_payload( ret = lttng_userspace_probe_location_function_set_binary_fd( *location, binary_fd); if (ret) { - const int close_ret = close(binary_fd); - - if (close_ret) { - PERROR("Failed to close userspace probe function binary fd"); - } ret = -LTTNG_ERR_INVALID; goto end; } ret = (int) expected_size; end: + fd_handle_put(binary_fd); free(function_name); free(binary_path); return ret; @@ -1280,7 +1267,7 @@ int lttng_userspace_probe_location_tracepoint_create_from_payload( char *probe_name = NULL, *provider_name = NULL, *binary_path = NULL; int ret = 0; size_t expected_size; - const int binary_fd = lttng_payload_view_pop_fd(view); + struct fd_handle *binary_fd = lttng_payload_view_pop_fd_handle(view); assert(location); @@ -1355,17 +1342,13 @@ int lttng_userspace_probe_location_tracepoint_create_from_payload( ret = lttng_userspace_probe_location_tracepoint_set_binary_fd( *location, binary_fd); if (ret) { - const int close_ret = close(binary_fd); - - if (close_ret) { - PERROR("Failed to close userspace probe tracepoint binary fd"); - } ret = -LTTNG_ERR_INVALID; goto end; } ret = (int) expected_size; end: + fd_handle_put(binary_fd); free(probe_name); free(provider_name); free(binary_path); @@ -1505,9 +1488,10 @@ end: return ret; } -LTTNG_HIDDEN +static int lttng_userspace_probe_location_function_set_binary_fd( - struct lttng_userspace_probe_location *location, int binary_fd) + struct lttng_userspace_probe_location *location, + struct fd_handle *binary_fd) { int ret = 0; struct lttng_userspace_probe_location_function *function_location; @@ -1517,23 +1501,16 @@ int lttng_userspace_probe_location_function_set_binary_fd( function_location = container_of(location, struct lttng_userspace_probe_location_function, parent); - if (function_location->binary_fd >= 0) { - ret = close(function_location->binary_fd); - if (ret) { - PERROR("close"); - ret = -LTTNG_ERR_INVALID; - goto end; - } - } - + fd_handle_put(function_location->binary_fd); + fd_handle_get(binary_fd); function_location->binary_fd = binary_fd; -end: return ret; } -LTTNG_HIDDEN +static int lttng_userspace_probe_location_tracepoint_set_binary_fd( - struct lttng_userspace_probe_location *location, int binary_fd) + struct lttng_userspace_probe_location *location, + struct fd_handle *binary_fd) { int ret = 0; struct lttng_userspace_probe_location_tracepoint *tracepoint_location; @@ -1543,17 +1520,9 @@ int lttng_userspace_probe_location_tracepoint_set_binary_fd( tracepoint_location = container_of(location, struct lttng_userspace_probe_location_tracepoint, parent); - if (tracepoint_location->binary_fd >= 0) { - ret = close(tracepoint_location->binary_fd); - if (ret) { - PERROR("close"); - ret = -LTTNG_ERR_INVALID; - goto end; - } - } - + fd_handle_put(tracepoint_location->binary_fd); + fd_handle_get(binary_fd); tracepoint_location->binary_fd = binary_fd; -end: return ret; } @@ -1637,7 +1606,7 @@ int lttng_userspace_probe_location_function_flatten( flat_probe.function_name = flat_probe_start + sizeof(flat_probe); flat_probe.binary_path = flat_probe.function_name + function_name_len; - flat_probe.binary_fd = -1; + flat_probe.binary_fd = NULL; ret = lttng_dynamic_buffer_append(buffer, &flat_probe, sizeof(flat_probe)); if (ret) { @@ -1772,7 +1741,7 @@ int lttng_userspace_probe_location_tracepoint_flatten( flat_probe.probe_name = flat_probe_start + sizeof(flat_probe); flat_probe.provider_name = flat_probe.probe_name + probe_name_len; flat_probe.binary_path = flat_probe.provider_name + provider_name_len; - flat_probe.binary_fd = -1; + flat_probe.binary_fd = NULL; ret = lttng_dynamic_buffer_append(buffer, &flat_probe, sizeof(flat_probe)); if (ret) { goto end; diff --git a/src/lib/lttng-ctl/lttng-ctl.c b/src/lib/lttng-ctl/lttng-ctl.c index b2f25ecd0..700a83a13 100644 --- a/src/lib/lttng-ctl/lttng-ctl.c +++ b/src/lib/lttng-ctl/lttng-ctl.c @@ -23,10 +23,12 @@ #include #include #include +#include #include #include #include #include +#include #include #include #include @@ -39,6 +41,7 @@ #include #include #include +#include #include "filter/filter-ast.h" #include "filter/filter-parser.h" @@ -627,9 +630,10 @@ int lttng_ctl_ask_sessiond_payload(struct lttng_payload_view *message, { int ret; struct lttcomm_lttng_msg llm; + const int fd_count = lttng_payload_view_get_fd_handle_count(message); assert(reply->buffer.size == 0); - assert(reply->_fds.size == 0); + assert(lttng_dynamic_pointer_array_get_count(&reply->_fd_handles) == 0); ret = connect_sessiond(); if (ret < 0) { @@ -648,10 +652,13 @@ int lttng_ctl_ask_sessiond_payload(struct lttng_payload_view *message, goto end; } - if (lttng_payload_view_get_fd_count(message) > 0) { - ret = lttcomm_send_fds_unix_sock(sessiond_socket, - (const int *) message->_fds.buffer.data, - lttng_dynamic_array_get_count(&message->_fds)); + if (fd_count > 0) { + ret = lttcomm_send_payload_view_fds_unix_sock(sessiond_socket, + message); + if (ret < 0) { + ret = -LTTNG_ERR_FATAL; + goto end; + } } /* Get header from data transmission */ @@ -685,19 +692,9 @@ int lttng_ctl_ask_sessiond_payload(struct lttng_payload_view *message, } if (llm.fd_count > 0) { - ret = lttng_dynamic_array_set_count(&reply->_fds, llm.fd_count); - if (ret) { - ret = -LTTNG_ERR_NOMEM; - goto end; - } - - ret = lttcomm_recv_fds_unix_sock(sessiond_socket, - (int *) reply->_fds.buffer.data, llm.fd_count); - if (ret > 0 && ret != llm.fd_count * sizeof(int)) { - ret = -LTTNG_ERR_INVALID_PROTOCOL; - goto end; - } else if (ret <= 0) { - ret = -LTTNG_ERR_FATAL; + ret = lttcomm_recv_payload_fds_unix_sock( + sessiond_socket, llm.fd_count, reply); + if (ret < 0) { goto end; } } @@ -1360,7 +1357,7 @@ int lttng_enable_event_with_exclusions(struct lttng_handle *handle, { struct lttng_payload_view view = lttng_payload_view_from_payload( &payload, 0, -1); - int fd_count = lttng_payload_view_get_fd_count(&view); + int fd_count = lttng_payload_view_get_fd_handle_count(&view); int fd_to_send; if (fd_count < 0) { @@ -1369,12 +1366,15 @@ int lttng_enable_event_with_exclusions(struct lttng_handle *handle, assert(fd_count == 0 || fd_count == 1); if (fd_count == 1) { - ret = lttng_payload_view_pop_fd(&view); - if (ret < 0) { + struct fd_handle *handle = + lttng_payload_view_pop_fd_handle(&view); + + if (!handle) { goto mem_error; } - fd_to_send = ret; + fd_to_send = fd_handle_get_fd(handle); + fd_handle_put(handle); } ret = lttng_ctl_ask_sessiond_fds_varlen(&lsm, diff --git a/tests/unit/test_payload.c b/tests/unit/test_payload.c index 11c57f21d..be97d4990 100644 --- a/tests/unit/test_payload.c +++ b/tests/unit/test_payload.c @@ -8,6 +8,7 @@ #include #include #include +#include static const int TEST_COUNT = 5; @@ -20,16 +21,28 @@ static void test_fd_push_pop_order(void) { int ret, i; struct lttng_payload payload; + int fds[3]; lttng_payload_init(&payload); diag("Validating fd push/pop order"); for (i = 0; i < 3; i++) { - ret = lttng_payload_push_fd(&payload, i); + int fd = eventfd(0, 0); + struct fd_handle *handle; + + assert(fd >= 0); + fds[i] = fd; + + handle = fd_handle_create(fd); + assert(handle); + + ret = lttng_payload_push_fd_handle(&payload, handle); + fd_handle_put(handle); if (ret) { break; } } + ok(ret == 0, "Added three file descriptors to an lttng_payload"); { @@ -39,8 +52,11 @@ static void test_fd_push_pop_order(void) &payload, 0, -1); for (i = 0; i < 3; i++) { - ret = lttng_payload_view_pop_fd(&view); - fail_pop |= ret != i; + struct fd_handle *handle = + lttng_payload_view_pop_fd_handle(&view); + + fail_pop |= fd_handle_get_fd(handle) != fds[i]; + fd_handle_put(handle); } ok(!fail_pop, "File descriptors are popped from a payload view in the order of insertion"); @@ -59,26 +75,38 @@ static void test_fd_push_pop_imbalance(void) diag("Validating fd pop imbalance"); for (i = 0; i < 10; i++) { - ret = lttng_payload_push_fd(&payload, i); + struct fd_handle *handle; + int fd = eventfd(0, 0); + + assert(fd >= 0); + + handle = fd_handle_create(fd); + assert(handle); + + ret = lttng_payload_push_fd_handle(&payload, handle); + fd_handle_put(handle); if (ret) { break; } } { + struct fd_handle *handle; struct lttng_payload_view view = lttng_payload_view_from_payload( &payload, 0, -1); for (i = 0; i < 10; i++) { - ret = lttng_payload_view_pop_fd(&view); - if (ret == -1) { + handle = lttng_payload_view_pop_fd_handle(&view); + fd_handle_put(handle); + if (!handle) { goto fail; } } - ret = lttng_payload_view_pop_fd(&view); - ok(ret == -1, test_description); + handle = lttng_payload_view_pop_fd_handle(&view); + ok(!handle, test_description); + fd_handle_put(handle); } lttng_payload_reset(&payload); @@ -91,53 +119,70 @@ fail: static void test_fd_pop_fd_root_views(void) { int ret, i; - const int fd = 42; + const int fd = eventfd(0, 0); + struct fd_handle *handle = fd_handle_create(fd); struct lttng_payload payload; const char * const test_description = "Same file descriptor returned when popping from different top-level views"; lttng_payload_init(&payload); + assert(handle); diag("Validating root view fd pop behaviour"); - ret = lttng_payload_push_fd(&payload, fd); + ret = lttng_payload_push_fd_handle(&payload, handle); if (ret) { goto fail; } for (i = 0; i < 5; i++) { + int view_fd; + struct fd_handle *view_handle; struct lttng_payload_view view = lttng_payload_view_from_payload( &payload, 0, -1); - ret = lttng_payload_view_pop_fd(&view); - if (ret != fd) { + view_handle = lttng_payload_view_pop_fd_handle(&view); + if (!view_handle) { + goto fail; + } + + view_fd = fd_handle_get_fd(view_handle); + fd_handle_put(view_handle); + if (view_fd != fd || view_handle != handle) { goto fail; } } lttng_payload_reset(&payload); pass(test_description); + fd_handle_put(handle); return; fail: lttng_payload_reset(&payload); fail(test_description); + fd_handle_put(handle); } static void test_fd_pop_fd_descendant_views(void) { int ret; const int fd1 = 42, fd2 = 1837; + struct fd_handle *handle1 = fd_handle_create(fd1); + struct fd_handle *handle2 = fd_handle_create(fd2); + struct fd_handle *view_handle1 = NULL, *view_handle2 = NULL; struct lttng_payload payload; const char * const test_description = "Different file descriptors returned when popping from descendant views"; lttng_payload_init(&payload); + assert(handle1); + assert(handle2); diag("Validating descendant view fd pop behaviour"); - ret = lttng_payload_push_fd(&payload, fd1); + ret = lttng_payload_push_fd_handle(&payload, handle1); if (ret) { goto fail; } - ret = lttng_payload_push_fd(&payload, fd2); + ret = lttng_payload_push_fd_handle(&payload, handle2); if (ret) { goto fail; } @@ -150,23 +195,31 @@ static void test_fd_pop_fd_descendant_views(void) lttng_payload_view_from_view( &view1, 0, -1); - ret = lttng_payload_view_pop_fd(&view1); - if (ret != fd1) { + view_handle1 = lttng_payload_view_pop_fd_handle(&view1); + if (!view_handle1 || fd_handle_get_fd(view_handle1) != fd1) { goto fail; } - ret = lttng_payload_view_pop_fd(&view2); - if (ret != fd2) { + view_handle2 = lttng_payload_view_pop_fd_handle(&view2); + if (!view_handle2 || fd_handle_get_fd(view_handle2) != fd2) { goto fail; } } lttng_payload_reset(&payload); pass(test_description); + fd_handle_put(handle1); + fd_handle_put(handle2); + fd_handle_put(view_handle1); + fd_handle_put(view_handle2); return; fail: lttng_payload_reset(&payload); fail(test_description); + fd_handle_put(handle1); + fd_handle_put(handle2); + fd_handle_put(view_handle1); + fd_handle_put(view_handle2); } int main(void) -- 2.34.1