From: Jérémie Galarneau Date: Fri, 29 Nov 2019 20:42:24 +0000 (-0500) Subject: Make lttng_directory_handle reference countable X-Git-Tag: v2.12.0-rc1~174 X-Git-Url: https://git.lttng.org./?a=commitdiff_plain;h=cbf53d23cecaca9c6ec71582663c4a8254a9f285;p=lttng-tools.git Make lttng_directory_handle reference countable Currently, the lttng_directory_handle API assumes that the caller has exclusive ownership of the lttng_directory_handle. Directory handles are passed by reference and populated by an "init" method and finalized using the "fini" method. In order to allow multiple references to a path or directory file descriptor in follow-up patches, the API is moved to a model where directory handles are dynamically allocated and released using the "put" method. No change in behaviour is intended by this change beyond adapting the API. The objective of the change is to make it easier to implement copy operations of trace chunks that are configured to use an fd-tracker and reduce the number of open file descriptors when lttng_directory_handles are copied. Signed-off-by: Jérémie Galarneau Change-Id: Ibc8e97ea9e949adafef44533c30b61e3a9fa1f7d --- diff --git a/src/bin/lttng-relayd/main.c b/src/bin/lttng-relayd/main.c index b3983b163..169832002 100644 --- a/src/bin/lttng-relayd/main.c +++ b/src/bin/lttng-relayd/main.c @@ -2437,7 +2437,7 @@ static int relay_create_trace_chunk(const struct lttcomm_relayd_hdr *recv_hdr, struct lttng_trace_chunk *chunk = NULL, *published_chunk = NULL; enum lttng_error_code reply_code = LTTNG_OK; enum lttng_trace_chunk_status chunk_status; - struct lttng_directory_handle session_output; + struct lttng_directory_handle *session_output = NULL; if (!session || !conn->version_check_done) { ERR("Trying to create a trace chunk before version check"); @@ -2512,14 +2512,15 @@ static int relay_create_trace_chunk(const struct lttcomm_relayd_hdr *recv_hdr, goto end; } - ret = session_init_output_directory_handle( - conn->session, &session_output); - if (ret) { + session_output = session_create_output_directory_handle( + conn->session); + if (!session_output) { reply_code = LTTNG_ERR_CREATE_DIR_FAIL; goto end; } - chunk_status = lttng_trace_chunk_set_as_owner(chunk, &session_output); - lttng_directory_handle_fini(&session_output); + chunk_status = lttng_trace_chunk_set_as_owner(chunk, session_output); + lttng_directory_handle_put(session_output); + session_output = NULL; if (chunk_status != LTTNG_TRACE_CHUNK_STATUS_OK) { reply_code = LTTNG_ERR_UNK; ret = -1; @@ -2575,6 +2576,7 @@ end: end_no_reply: lttng_trace_chunk_put(chunk); lttng_trace_chunk_put(published_chunk); + lttng_directory_handle_put(session_output); return ret; } diff --git a/src/bin/lttng-relayd/session.c b/src/bin/lttng-relayd/session.c index 8f61e715f..df6bc102a 100644 --- a/src/bin/lttng-relayd/session.c +++ b/src/bin/lttng-relayd/session.c @@ -185,10 +185,10 @@ static int session_set_anonymous_chunk(struct relay_session *session) int ret = 0; struct lttng_trace_chunk *chunk = NULL; enum lttng_trace_chunk_status status; - struct lttng_directory_handle output_directory; + struct lttng_directory_handle *output_directory; - ret = session_init_output_directory_handle(session, &output_directory); - if (ret) { + output_directory = session_create_output_directory_handle(session); + if (!output_directory) { goto end; } @@ -203,16 +203,17 @@ static int session_set_anonymous_chunk(struct relay_session *session) goto end; } - status = lttng_trace_chunk_set_as_owner(chunk, &output_directory); + status = lttng_trace_chunk_set_as_owner(chunk, output_directory); if (status != LTTNG_TRACE_CHUNK_STATUS_OK) { ret = -1; goto end; } + output_directory = NULL; session->current_trace_chunk = chunk; chunk = NULL; end: lttng_trace_chunk_put(chunk); - lttng_directory_handle_fini(&output_directory); + lttng_directory_handle_put(output_directory); return ret; } @@ -531,8 +532,8 @@ void print_sessions(void) rcu_read_unlock(); } -int session_init_output_directory_handle(struct relay_session *session, - struct lttng_directory_handle *handle) +struct lttng_directory_handle *session_create_output_directory_handle( + struct relay_session *session) { int ret; /* @@ -540,11 +541,11 @@ int session_init_output_directory_handle(struct relay_session *session, * e.g. /home/user/lttng-traces/hostname/session_name */ char *full_session_path = NULL; + struct lttng_directory_handle *handle = NULL; pthread_mutex_lock(&session->lock); full_session_path = create_output_path(session->output_path); if (!full_session_path) { - ret = -1; goto end; } @@ -556,12 +557,9 @@ int session_init_output_directory_handle(struct relay_session *session, goto end; } - ret = lttng_directory_handle_init(handle, full_session_path); - if (ret) { - goto end; - } + handle = lttng_directory_handle_create(full_session_path); end: pthread_mutex_unlock(&session->lock); free(full_session_path); - return ret; + return handle; } diff --git a/src/bin/lttng-relayd/session.h b/src/bin/lttng-relayd/session.h index 3f3ac02af..aa2a58a98 100644 --- a/src/bin/lttng-relayd/session.h +++ b/src/bin/lttng-relayd/session.h @@ -155,8 +155,8 @@ void session_put(struct relay_session *session); int session_close(struct relay_session *session); int session_abort(struct relay_session *session); -int session_init_output_directory_handle(struct relay_session *session, - struct lttng_directory_handle *handle); +struct lttng_directory_handle *session_create_output_directory_handle( + struct relay_session *session); void print_sessions(void); diff --git a/src/bin/lttng-sessiond/consumer.c b/src/bin/lttng-sessiond/consumer.c index d376c9ee3..c4cfb2fd7 100644 --- a/src/bin/lttng-sessiond/consumer.c +++ b/src/bin/lttng-sessiond/consumer.c @@ -1830,7 +1830,7 @@ int consumer_create_trace_chunk(struct consumer_socket *socket, msg.u.create_trace_chunk.chunk_id = chunk_id; if (chunk_has_local_output) { - chunk_status = lttng_trace_chunk_get_chunk_directory_handle( + chunk_status = lttng_trace_chunk_borrow_chunk_directory_handle( chunk, &chunk_directory_handle); if (chunk_status != LTTNG_TRACE_CHUNK_STATUS_OK) { ret = -LTTNG_ERR_FATAL; diff --git a/src/bin/lttng-sessiond/session.c b/src/bin/lttng-sessiond/session.c index a673fe31b..7ef188049 100644 --- a/src/bin/lttng-sessiond/session.c +++ b/src/bin/lttng-sessiond/session.c @@ -575,7 +575,7 @@ struct lttng_trace_chunk *session_create_new_trace_chunk( const time_t chunk_creation_ts = time(NULL); bool is_local_trace; const char *base_path; - struct lttng_directory_handle session_output_directory; + struct lttng_directory_handle *session_output_directory = NULL; const struct lttng_credentials session_credentials = { .uid = session->uid, .gid = session->gid, @@ -640,20 +640,21 @@ struct lttng_trace_chunk *session_create_new_trace_chunk( if (ret) { goto error; } - ret = lttng_directory_handle_init(&session_output_directory, - base_path); - if (ret) { + session_output_directory = lttng_directory_handle_create(base_path); + if (!session_output_directory) { goto error; } chunk_status = lttng_trace_chunk_set_as_owner(trace_chunk, - &session_output_directory); - lttng_directory_handle_fini(&session_output_directory); + session_output_directory); + lttng_directory_handle_put(session_output_directory); + session_output_directory = NULL; if (chunk_status != LTTNG_TRACE_CHUNK_STATUS_OK) { goto error; } end: return trace_chunk; error: + lttng_directory_handle_put(session_output_directory); lttng_trace_chunk_put(trace_chunk); trace_chunk = NULL; goto end; diff --git a/src/common/compat/directory-handle.c b/src/common/compat/directory-handle.c index 95be9ec3d..d9d1e2337 100644 --- a/src/common/compat/directory-handle.c +++ b/src/common/compat/directory-handle.c @@ -88,62 +88,79 @@ int _run_as_rmdir_recursive( uid_t uid, gid_t gid, int flags); static void lttng_directory_handle_invalidate(struct lttng_directory_handle *handle); +static +void lttng_directory_handle_release(struct urcu_ref *ref); #ifdef COMPAT_DIRFD LTTNG_HIDDEN -int lttng_directory_handle_init(struct lttng_directory_handle *new_handle, - const char *path) +struct lttng_directory_handle *lttng_directory_handle_create(const char *path) { const struct lttng_directory_handle cwd_handle = { .dirfd = AT_FDCWD, }; /* Open a handle to the CWD if NULL is passed. */ - return lttng_directory_handle_init_from_handle(new_handle, - path, - &cwd_handle); + return lttng_directory_handle_create_from_handle(path, &cwd_handle); } LTTNG_HIDDEN -int lttng_directory_handle_init_from_handle( - struct lttng_directory_handle *new_handle, const char *path, - const struct lttng_directory_handle *handle) +struct lttng_directory_handle *lttng_directory_handle_create_from_handle( + const char *path, + const struct lttng_directory_handle *ref_handle) { - int ret; + int dirfd; + struct lttng_directory_handle *handle = NULL; if (!path) { - ret = lttng_directory_handle_copy(handle, new_handle); + handle = lttng_directory_handle_copy(ref_handle); goto end; } if (!*path) { ERR("Failed to initialize directory handle: provided path is an empty string"); - ret = -1; goto end; } - ret = openat(handle->dirfd, path, O_RDONLY | O_DIRECTORY | O_CLOEXEC); - if (ret == -1) { + + dirfd = openat(ref_handle->dirfd, path, O_RDONLY | O_DIRECTORY | O_CLOEXEC); + if (dirfd == -1) { PERROR("Failed to initialize directory handle to \"%s\"", path); goto end; } - new_handle->dirfd = ret; - ret = 0; + + handle = lttng_directory_handle_create_from_dirfd(dirfd); + if (!handle) { + goto error_close; + } end: - return ret; + return handle; +error_close: + if (close(dirfd)) { + PERROR("Failed to close directory file descriptor"); + } + return NULL; } LTTNG_HIDDEN -int lttng_directory_handle_init_from_dirfd( - struct lttng_directory_handle *handle, int dirfd) +struct lttng_directory_handle *lttng_directory_handle_create_from_dirfd( + int dirfd) { + struct lttng_directory_handle *handle = zmalloc(sizeof(*handle)); + + if (!handle) { + goto end; + } handle->dirfd = dirfd; - return 0; + urcu_ref_init(&handle->ref); +end: + return handle; } -LTTNG_HIDDEN -void lttng_directory_handle_fini(struct lttng_directory_handle *handle) +static +void lttng_directory_handle_release(struct urcu_ref *ref) { int ret; + struct lttng_directory_handle *handle = + container_of(ref, struct lttng_directory_handle, ref); if (handle->dirfd == AT_FDCWD || handle->dirfd == -1) { goto end; @@ -151,28 +168,35 @@ void lttng_directory_handle_fini(struct lttng_directory_handle *handle) ret = close(handle->dirfd); if (ret == -1) { PERROR("Failed to close directory file descriptor of directory handle"); - abort(); } end: lttng_directory_handle_invalidate(handle); + free(handle); } LTTNG_HIDDEN -int lttng_directory_handle_copy(const struct lttng_directory_handle *handle, - struct lttng_directory_handle *new_copy) +struct lttng_directory_handle *lttng_directory_handle_copy( + const struct lttng_directory_handle *handle) { - int ret = 0; + struct lttng_directory_handle *new_handle = NULL; if (handle->dirfd == AT_FDCWD) { - new_copy->dirfd = handle->dirfd; + new_handle = lttng_directory_handle_create_from_dirfd(AT_FDCWD); } else { - new_copy->dirfd = dup(handle->dirfd); - if (new_copy->dirfd == -1) { - PERROR("Failed to duplicate directory fd of directory handle"); - ret = -1; + const int new_dirfd = dup(handle->dirfd); + + if (new_dirfd == -1) { + PERROR("Failed to duplicate directory file descriptor of directory handle"); + goto end; + } + new_handle = lttng_directory_handle_create_from_dirfd( + new_dirfd); + if (!new_handle && close(new_dirfd)) { + PERROR("Failed to close directory file descriptor of directory handle"); } } - return ret; +end: + return new_handle; } static @@ -343,8 +367,22 @@ end: return ret; } +static +struct lttng_directory_handle *_lttng_directory_handle_create(char *path) +{ + struct lttng_directory_handle *handle = zmalloc(sizeof(*handle)); + + if (!handle) { + goto end; + } + urcu_ref_init(&handle->ref); + handle->base_path = path; +end: + return handle; +} + LTTNG_HIDDEN -int lttng_directory_handle_init(struct lttng_directory_handle *handle, +struct lttng_directory_handle *lttng_directory_handle_create( const char *path) { int ret; @@ -352,6 +390,7 @@ int lttng_directory_handle_init(struct lttng_directory_handle *handle, size_t cwd_len, path_len; char cwd_buf[LTTNG_PATH_MAX] = {}; char handle_buf[LTTNG_PATH_MAX] = {}; + struct lttng_directory_handle *new_handle = NULL; bool add_cwd_slash = false, add_trailing_slash = false; const struct lttng_directory_handle cwd_handle = { .base_path = handle_buf, @@ -385,25 +424,26 @@ int lttng_directory_handle_init(struct lttng_directory_handle *handle, goto end; } - ret = lttng_directory_handle_init_from_handle(handle, path, - &cwd_handle); + new_handle = lttng_directory_handle_create_from_handle(path, &cwd_handle); end: - return ret; + return new_handle; } LTTNG_HIDDEN -int lttng_directory_handle_init_from_handle( - struct lttng_directory_handle *new_handle, const char *path, - const struct lttng_directory_handle *handle) +struct lttng_directory_handle *lttng_directory_handle_create_from_handle( + const char *path, + const struct lttng_directory_handle *ref_handle) { int ret; size_t path_len, handle_path_len; bool add_trailing_slash; struct stat stat_buf; + struct lttng_directory_handle *new_handle = NULL; + char *new_path = NULL; - assert(handle && handle->base_path); + assert(ref_handle && ref_handle->base_path); - ret = lttng_directory_handle_stat(handle, path, &stat_buf); + ret = lttng_directory_handle_stat(ref_handle, path, &stat_buf); if (ret == -1) { PERROR("Failed to create directory handle"); goto end; @@ -411,7 +451,7 @@ int lttng_directory_handle_init_from_handle( char full_path[LTTNG_PATH_MAX]; /* Best effort for logging purposes. */ - ret = get_full_path(handle, path, full_path, + ret = get_full_path(ref_handle, path, full_path, sizeof(full_path)); if (ret) { full_path[0] = '\0'; @@ -419,11 +459,10 @@ int lttng_directory_handle_init_from_handle( ERR("Failed to initialize directory handle to \"%s\": not a directory", full_path); - ret = -1; goto end; } if (!path) { - ret = lttng_directory_handle_copy(handle, new_handle); + new_handle = lttng_directory_handle_copy(ref_handle); goto end; } @@ -434,62 +473,81 @@ int lttng_directory_handle_init_from_handle( goto end; } if (*path == '/') { - new_handle->base_path = strdup(path); - ret = new_handle->base_path ? 0 : -1; + new_path = strdup(path); + if (!new_path) { + goto end; + } + /* Takes ownership of new_path. */ + new_handle = _lttng_directory_handle_create(new_path); + new_path = NULL; goto end; } add_trailing_slash = path[path_len - 1] != '/'; - handle_path_len = strlen(handle->base_path) + path_len + + handle_path_len = strlen(ref_handle->base_path) + path_len + !!add_trailing_slash; if (handle_path_len >= LTTNG_PATH_MAX) { ERR("Failed to initialize directory handle as the resulting path's length (%zu bytes) exceeds the maximal allowed length (%d bytes)", handle_path_len, LTTNG_PATH_MAX); - ret = -1; goto end; } - new_handle->base_path = zmalloc(handle_path_len); - if (!new_handle->base_path) { + new_path = zmalloc(handle_path_len); + if (!new_path) { PERROR("Failed to initialize directory handle"); - ret = -1; goto end; } ret = sprintf(new_handle->base_path, "%s%s%s", - handle->base_path, + ref_handle->base_path, path, add_trailing_slash ? "/" : ""); if (ret == -1 || ret >= handle_path_len) { ERR("Failed to initialize directory handle: path formatting failed"); - ret = -1; goto end; } + new_handle = _lttng_directory_handle_create(new_path); + new_path = NULL; end: - return ret; + free(new_path); + return new_handle; } LTTNG_HIDDEN -int lttng_directory_handle_init_from_dirfd( - struct lttng_directory_handle *handle, int dirfd) +struct lttng_directory_handle *lttng_directory_handle_create_from_dirfd( + int dirfd) { assert(dirfd == AT_FDCWD); - return lttng_directory_handle_init(handle, NULL); + return lttng_directory_handle_create(NULL); } -LTTNG_HIDDEN -void lttng_directory_handle_fini(struct lttng_directory_handle *handle) +static +void lttng_directory_handle_release(struct urcu_ref *ref) { + struct lttng_directory_handle *handle = + container_of(ref, struct lttng_directory_handle, ref); + free(handle->base_path); lttng_directory_handle_invalidate(handle); + free(handle); } LTTNG_HIDDEN -int lttng_directory_handle_copy(const struct lttng_directory_handle *handle, - struct lttng_directory_handle *new_copy) +struct lttng_directory_handle *lttng_directory_handle_copy( + const struct lttng_directory_handle *handle) { - new_copy->base_path = strdup(handle->base_path); - return new_copy->base_path ? 0 : -1; + struct lttng_directory_handle *new_handle = NULL; + char *new_path = NULL; + + if (handle->base_path) { + new_path = strdup(handle->base_path); + if (!new_path) { + goto end; + } + } + new_handle = _lttng_directory_handle_create(new_path); +end: + return new_handle; } static @@ -819,16 +877,6 @@ end: return ret; } -LTTNG_HIDDEN -struct lttng_directory_handle -lttng_directory_handle_move(struct lttng_directory_handle *original) -{ - const struct lttng_directory_handle tmp = *original; - - lttng_directory_handle_invalidate(original); - return tmp; -} - static int create_directory_recursive(const struct lttng_directory_handle *handle, const char *path, mode_t mode) @@ -884,6 +932,22 @@ error: return ret; } +LTTNG_HIDDEN +bool lttng_directory_handle_get(struct lttng_directory_handle *handle) +{ + return urcu_ref_get_unless_zero(&handle->ref); +} + +LTTNG_HIDDEN +void lttng_directory_handle_put(struct lttng_directory_handle *handle) +{ + if (!handle) { + return; + } + assert(handle->ref.refcount); + urcu_ref_put(&handle->ref, lttng_directory_handle_release); +} + LTTNG_HIDDEN int lttng_directory_handle_create_subdirectory_as_user( const struct lttng_directory_handle *handle, diff --git a/src/common/compat/directory-handle.h b/src/common/compat/directory-handle.h index 26da76bcc..5974e36e3 100644 --- a/src/common/compat/directory-handle.h +++ b/src/common/compat/directory-handle.h @@ -20,6 +20,7 @@ #include #include +#include enum lttng_directory_handle_rmdir_recursive_flags { LTTNG_DIRECTORY_HANDLE_FAIL_NON_EMPTY_FLAG = (1U << 0), @@ -35,6 +36,7 @@ enum lttng_directory_handle_rmdir_recursive_flags { */ #ifdef COMPAT_DIRFD struct lttng_directory_handle { + struct urcu_ref ref; int dirfd; }; @@ -47,23 +49,24 @@ int lttng_directory_handle_get_dirfd( #else struct lttng_directory_handle { + struct urcu_ref ref; char *base_path; }; #endif /* - * Initialize a directory handle to the provided path. Passing a NULL path + * Create a directory handle to the provided path. Passing a NULL path * returns a handle to the current working directory. * - * An initialized directory handle must be finalized using - * lttng_directory_handle_fini(). + * The reference to the directory handle must be released using + * lttng_directory_handle_put(). */ LTTNG_HIDDEN -int lttng_directory_handle_init(struct lttng_directory_handle *handle, +struct lttng_directory_handle *lttng_directory_handle_create( const char *path); /* - * Initialize a new directory handle to a path relative to an existing handle. + * Create a new directory handle to a path relative to an existing handle. * * The provided path must already exist. Note that the creation of a * subdirectory and the creation of a handle are kept as separate operations @@ -72,53 +75,49 @@ int lttng_directory_handle_init(struct lttng_directory_handle *handle, * * Passing a NULL path effectively copies the original handle. * - * An initialized directory handle must be finalized using - * lttng_directory_handle_fini(). + * The reference to the directory handle must be released using + * lttng_directory_handle_put(). */ LTTNG_HIDDEN -int lttng_directory_handle_init_from_handle( - struct lttng_directory_handle *new_handle, +struct lttng_directory_handle *lttng_directory_handle_create_from_handle( const char *path, - const struct lttng_directory_handle *handle); + const struct lttng_directory_handle *ref_handle); /* - * Initialize a new directory handle from an existing directory fd. + * Create a new directory handle from an existing directory fd. * * The new directory handle assumes the ownership of the directory fd. * Note that this method should only be used in very specific cases, such as * re-creating a directory handle from a dirfd passed over a unix socket. * - * An initialized directory handle must be finalized using - * lttng_directory_handle_fini(). + * The reference to the directory handle must be released using + * lttng_directory_handle_put(). */ LTTNG_HIDDEN -int lttng_directory_handle_init_from_dirfd( - struct lttng_directory_handle *handle, int dirfd); +struct lttng_directory_handle *lttng_directory_handle_create_from_dirfd( + int dirfd); /* * Copy a directory handle. + * + * The reference to the directory handle must be released using + * lttng_directory_handle_put(). */ LTTNG_HIDDEN -int lttng_directory_handle_copy(const struct lttng_directory_handle *handle, - struct lttng_directory_handle *new_copy); +struct lttng_directory_handle *lttng_directory_handle_copy( + const struct lttng_directory_handle *handle); /* - * Move a directory handle. The original directory handle may no longer be - * used after this call. This call cannot fail; directly assign the - * return value to the new directory handle. - * - * It is safe (but unnecessary) to call lttng_directory_handle_fini on the - * original handle. + * Acquire a reference to a directory handle. */ LTTNG_HIDDEN -struct lttng_directory_handle -lttng_directory_handle_move(struct lttng_directory_handle *original); +bool lttng_directory_handle_get(struct lttng_directory_handle *handle); /* - * Release the resources of a directory handle. + * Release a reference to a directory handle. */ LTTNG_HIDDEN -void lttng_directory_handle_fini(struct lttng_directory_handle *handle); +void lttng_directory_handle_put(struct lttng_directory_handle *handle); /* * Create a subdirectory relative to a directory handle. diff --git a/src/common/consumer/consumer.c b/src/common/consumer/consumer.c index 3c51cfa6f..00eb35670 100644 --- a/src/common/consumer/consumer.c +++ b/src/common/consumer/consumer.c @@ -4463,6 +4463,7 @@ enum lttcomm_return_code lttng_consumer_create_trace_chunk( */ chunk_status = lttng_trace_chunk_set_as_user(created_chunk, chunk_directory_handle); + chunk_directory_handle = NULL; if (chunk_status != LTTNG_TRACE_CHUNK_STATUS_OK) { ERR("Failed to set trace chunk's directory handle"); ret_code = LTTCOMM_CONSUMERD_CREATE_TRACE_CHUNK_FAILED; diff --git a/src/common/kernel-consumer/kernel-consumer.c b/src/common/kernel-consumer/kernel-consumer.c index 9fa92f6b8..fdd9ca65b 100644 --- a/src/common/kernel-consumer/kernel-consumer.c +++ b/src/common/kernel-consumer/kernel-consumer.c @@ -1185,8 +1185,7 @@ error_rotate_channel: *msg.u.create_trace_chunk.override_name ? msg.u.create_trace_chunk.override_name : NULL; - LTTNG_OPTIONAL(struct lttng_directory_handle) chunk_directory_handle = - LTTNG_OPTIONAL_INIT; + struct lttng_directory_handle *chunk_directory_handle = NULL; /* * The session daemon will only provide a chunk directory file @@ -1211,17 +1210,15 @@ error_rotate_channel: DBG("Received trace chunk directory fd (%d)", chunk_dirfd); - ret = lttng_directory_handle_init_from_dirfd( - &chunk_directory_handle.value, + chunk_directory_handle = lttng_directory_handle_create_from_dirfd( chunk_dirfd); - if (ret) { + if (!chunk_directory_handle) { ERR("Failed to initialize chunk directory handle from directory file descriptor"); if (close(chunk_dirfd)) { PERROR("Failed to close chunk directory file descriptor"); } goto error_fatal; } - chunk_directory_handle.is_set = true; } ret_code = lttng_consumer_create_trace_chunk( @@ -1234,14 +1231,8 @@ error_rotate_channel: msg.u.create_trace_chunk.credentials.is_set ? &credentials : NULL, - chunk_directory_handle.is_set ? - &chunk_directory_handle.value : - NULL); - - if (chunk_directory_handle.is_set) { - lttng_directory_handle_fini( - &chunk_directory_handle.value); - } + chunk_directory_handle); + lttng_directory_handle_put(chunk_directory_handle); goto end_msg_sessiond; } case LTTNG_CONSUMER_CLOSE_TRACE_CHUNK: diff --git a/src/common/runas.c b/src/common/runas.c index 222047cb6..71d4016c8 100644 --- a/src/common/runas.c +++ b/src/common/runas.c @@ -351,22 +351,27 @@ int _mkdirat_recursive(struct run_as_data *data, struct run_as_ret *ret_value) { const char *path; mode_t mode; - struct lttng_directory_handle handle; + struct lttng_directory_handle *handle; path = data->u.mkdir.path; mode = data->u.mkdir.mode; - (void) lttng_directory_handle_init_from_dirfd(&handle, - data->u.mkdir.dirfd); + handle = lttng_directory_handle_create_from_dirfd(data->u.mkdir.dirfd); + if (!handle) { + ret_value->_errno = errno; + ret_value->_error = true; + ret_value->u.ret = -1; + goto end; + } /* Ownership of dirfd is transferred to the handle. */ data->u.mkdir.dirfd = -1; /* Safe to call as we have transitioned to the requested uid/gid. */ - ret_value->u.ret = - lttng_directory_handle_create_subdirectory_recursive( - &handle, path, mode); + ret_value->u.ret = lttng_directory_handle_create_subdirectory_recursive( + handle, path, mode); ret_value->_errno = errno; ret_value->_error = (ret_value->u.ret) ? true : false; - lttng_directory_handle_fini(&handle); + lttng_directory_handle_put(handle); +end: return ret_value->u.ret; } @@ -375,22 +380,27 @@ int _mkdirat(struct run_as_data *data, struct run_as_ret *ret_value) { const char *path; mode_t mode; - struct lttng_directory_handle handle; + struct lttng_directory_handle *handle; path = data->u.mkdir.path; mode = data->u.mkdir.mode; - (void) lttng_directory_handle_init_from_dirfd(&handle, - data->u.mkdir.dirfd); + handle = lttng_directory_handle_create_from_dirfd(data->u.mkdir.dirfd); + if (!handle) { + ret_value->u.ret = -1; + ret_value->_errno = errno; + ret_value->_error = true; + goto end; + } /* Ownership of dirfd is transferred to the handle. */ data->u.mkdir.dirfd = -1; /* Safe to call as we have transitioned to the requested uid/gid. */ - ret_value->u.ret = - lttng_directory_handle_create_subdirectory( - &handle, path, mode); + ret_value->u.ret = lttng_directory_handle_create_subdirectory( + handle, path, mode); ret_value->_errno = errno; ret_value->_error = (ret_value->u.ret) ? true : false; - lttng_directory_handle_fini(&handle); + lttng_directory_handle_put(handle); +end: return ret_value->u.ret; } @@ -398,14 +408,19 @@ static int _open(struct run_as_data *data, struct run_as_ret *ret_value) { int fd; - struct lttng_directory_handle handle; + struct lttng_directory_handle *handle; - (void) lttng_directory_handle_init_from_dirfd(&handle, - data->u.open.dirfd); + handle = lttng_directory_handle_create_from_dirfd(data->u.open.dirfd); + if (!handle) { + ret_value->_errno = errno; + ret_value->_error = true; + ret_value->u.ret = -1; + goto end; + } /* Ownership of dirfd is transferred to the handle. */ data->u.open.dirfd = -1; - fd = lttng_directory_handle_open_file(&handle, + fd = lttng_directory_handle_open_file(handle, data->u.open.path, data->u.open.flags, data->u.open.mode); if (fd < 0) { @@ -418,64 +433,83 @@ int _open(struct run_as_data *data, struct run_as_ret *ret_value) ret_value->_errno = errno; ret_value->_error = fd < 0; - lttng_directory_handle_fini(&handle); + lttng_directory_handle_put(handle); +end: return ret_value->u.ret; } static int _unlink(struct run_as_data *data, struct run_as_ret *ret_value) { - struct lttng_directory_handle handle; + struct lttng_directory_handle *handle; - (void) lttng_directory_handle_init_from_dirfd(&handle, - data->u.unlink.dirfd); + handle = lttng_directory_handle_create_from_dirfd(data->u.unlink.dirfd); + if (!handle) { + ret_value->u.ret = -1; + ret_value->_errno = errno; + ret_value->_error = true; + goto end; + } /* Ownership of dirfd is transferred to the handle. */ data->u.unlink.dirfd = -1; - ret_value->u.ret = lttng_directory_handle_unlink_file(&handle, + ret_value->u.ret = lttng_directory_handle_unlink_file(handle, data->u.unlink.path); ret_value->_errno = errno; ret_value->_error = (ret_value->u.ret) ? true : false; - lttng_directory_handle_fini(&handle); + lttng_directory_handle_put(handle); +end: return ret_value->u.ret; } static int _rmdir(struct run_as_data *data, struct run_as_ret *ret_value) { - struct lttng_directory_handle handle; + struct lttng_directory_handle *handle; - (void) lttng_directory_handle_init_from_dirfd(&handle, - data->u.rmdir.dirfd); + handle = lttng_directory_handle_create_from_dirfd(data->u.rmdir.dirfd); + if (!handle) { + ret_value->u.ret = -1; + ret_value->_errno = errno; + ret_value->_error = true; + goto end; + } /* Ownership of dirfd is transferred to the handle. */ data->u.rmdir.dirfd = -1; ret_value->u.ret = lttng_directory_handle_remove_subdirectory( - &handle, data->u.rmdir.path); + handle, data->u.rmdir.path); ret_value->_errno = errno; ret_value->_error = (ret_value->u.ret) ? true : false; - lttng_directory_handle_fini(&handle); + lttng_directory_handle_put(handle); +end: return ret_value->u.ret; } static int _rmdir_recursive(struct run_as_data *data, struct run_as_ret *ret_value) { - struct lttng_directory_handle handle; + struct lttng_directory_handle *handle; - (void) lttng_directory_handle_init_from_dirfd(&handle, - data->u.rmdir.dirfd); + handle = lttng_directory_handle_create_from_dirfd(data->u.rmdir.dirfd); + if (!handle) { + ret_value->u.ret = -1; + ret_value->_errno = errno; + ret_value->_error = true; + goto end; + } /* Ownership of dirfd is transferred to the handle. */ data->u.rmdir.dirfd = -1; ret_value->u.ret = lttng_directory_handle_remove_subdirectory_recursive( - &handle, data->u.rmdir.path, data->u.rmdir.flags); + handle, data->u.rmdir.path, data->u.rmdir.flags); ret_value->_errno = errno; ret_value->_error = (ret_value->u.ret) ? true : false; - lttng_directory_handle_fini(&handle); + lttng_directory_handle_put(handle); +end: return ret_value->u.ret; } @@ -483,26 +517,35 @@ static int _rename(struct run_as_data *data, struct run_as_ret *ret_value) { const char *old_path, *new_path; - struct lttng_directory_handle old_handle, new_handle; + struct lttng_directory_handle *old_handle = NULL, *new_handle = NULL; old_path = data->u.rename.old_path; new_path = data->u.rename.new_path; - (void) lttng_directory_handle_init_from_dirfd(&old_handle, + old_handle = lttng_directory_handle_create_from_dirfd( data->u.rename.dirfds[0]); - (void) lttng_directory_handle_init_from_dirfd(&new_handle, + if (!old_handle) { + ret_value->u.ret = -1; + goto end; + } + new_handle = lttng_directory_handle_create_from_dirfd( data->u.rename.dirfds[1]); + if (!new_handle) { + ret_value->u.ret = -1; + goto end; + } /* Ownership of dirfds are transferred to the handles. */ data->u.rename.dirfds[0] = data->u.rename.dirfds[1] = -1; /* Safe to call as we have transitioned to the requested uid/gid. */ ret_value->u.ret = lttng_directory_handle_rename( - &old_handle, old_path, &new_handle, new_path); + old_handle, old_path, new_handle, new_path); +end: + lttng_directory_handle_put(old_handle); + lttng_directory_handle_put(new_handle); ret_value->_errno = errno; ret_value->_error = (ret_value->u.ret) ? true : false; - lttng_directory_handle_fini(&old_handle); - lttng_directory_handle_fini(&new_handle); return ret_value->u.ret; } diff --git a/src/common/trace-chunk.c b/src/common/trace-chunk.c index 944743e0b..6181520fa 100644 --- a/src/common/trace-chunk.c +++ b/src/common/trace-chunk.c @@ -87,8 +87,8 @@ struct lttng_trace_chunk { LTTNG_OPTIONAL(time_t) timestamp_creation; LTTNG_OPTIONAL(time_t) timestamp_close; LTTNG_OPTIONAL(struct chunk_credentials) credentials; - LTTNG_OPTIONAL(struct lttng_directory_handle) session_output_directory; - LTTNG_OPTIONAL(struct lttng_directory_handle) chunk_directory; + struct lttng_directory_handle *session_output_directory; + struct lttng_directory_handle *chunk_directory; LTTNG_OPTIONAL(enum lttng_trace_chunk_command_type) close_command; }; @@ -221,12 +221,14 @@ void lttng_trace_chunk_init(struct lttng_trace_chunk *chunk) static void lttng_trace_chunk_fini(struct lttng_trace_chunk *chunk) { - if (chunk->session_output_directory.is_set) { - lttng_directory_handle_fini( - &chunk->session_output_directory.value); + if (chunk->session_output_directory) { + lttng_directory_handle_put( + chunk->session_output_directory); + chunk->session_output_directory = NULL; } - if (chunk->chunk_directory.is_set) { - lttng_directory_handle_fini(&chunk->chunk_directory.value); + if (chunk->chunk_directory) { + lttng_directory_handle_put(chunk->chunk_directory); + chunk->chunk_directory = NULL; } free(chunk->name); chunk->name = NULL; @@ -343,23 +345,20 @@ struct lttng_trace_chunk *lttng_trace_chunk_copy( new_chunk->timestamp_creation = source_chunk->timestamp_creation; new_chunk->timestamp_close = source_chunk->timestamp_close; new_chunk->credentials = source_chunk->credentials; - if (source_chunk->session_output_directory.is_set) { - if (lttng_directory_handle_copy( - &source_chunk->session_output_directory.value, - &new_chunk->session_output_directory.value)) { - goto error_unlock; - } else { - new_chunk->session_output_directory.is_set = true; - } + if (source_chunk->session_output_directory) { + const bool reference_acquired = lttng_directory_handle_get( + source_chunk->session_output_directory); + + assert(reference_acquired); + new_chunk->session_output_directory = + source_chunk->session_output_directory; } - if (source_chunk->chunk_directory.is_set) { - if (lttng_directory_handle_copy( - &source_chunk->chunk_directory.value, - &new_chunk->chunk_directory.value)) { - goto error_unlock; - } else { - new_chunk->chunk_directory.is_set = true; - } + if (source_chunk->chunk_directory) { + const bool reference_acquired = lttng_directory_handle_get( + source_chunk->chunk_directory); + + assert(reference_acquired); + new_chunk->chunk_directory = source_chunk->chunk_directory; } new_chunk->close_command = source_chunk->close_command; pthread_mutex_unlock(&source_chunk->lock); @@ -603,7 +602,8 @@ enum lttng_trace_chunk_status lttng_trace_chunk_set_as_owner( { int ret; enum lttng_trace_chunk_status status = LTTNG_TRACE_CHUNK_STATUS_OK; - struct lttng_directory_handle chunk_directory_handle; + struct lttng_directory_handle *chunk_directory_handle = NULL; + bool reference_acquired; pthread_mutex_lock(&chunk->lock); if (chunk->mode.is_set) { @@ -638,18 +638,20 @@ enum lttng_trace_chunk_status lttng_trace_chunk_set_as_owner( goto end; } } - ret = lttng_directory_handle_init_from_handle(&chunk_directory_handle, + chunk_directory_handle = lttng_directory_handle_create_from_handle( chunk->name, session_output_directory); - if (ret) { + if (!chunk_directory_handle) { /* The function already logs on all error paths. */ status = LTTNG_TRACE_CHUNK_STATUS_ERROR; goto end; } - LTTNG_OPTIONAL_SET(&chunk->session_output_directory, - lttng_directory_handle_move(session_output_directory)); - LTTNG_OPTIONAL_SET(&chunk->chunk_directory, - lttng_directory_handle_move(&chunk_directory_handle)); + chunk->chunk_directory = chunk_directory_handle; + chunk_directory_handle = NULL; + reference_acquired = lttng_directory_handle_get( + session_output_directory); + assert(reference_acquired); + chunk->session_output_directory = session_output_directory; LTTNG_OPTIONAL_SET(&chunk->mode, TRACE_CHUNK_MODE_OWNER); end: pthread_mutex_unlock(&chunk->lock); @@ -662,6 +664,7 @@ enum lttng_trace_chunk_status lttng_trace_chunk_set_as_user( struct lttng_directory_handle *chunk_directory) { enum lttng_trace_chunk_status status = LTTNG_TRACE_CHUNK_STATUS_OK; + bool reference_acquired; pthread_mutex_lock(&chunk->lock); if (chunk->mode.is_set) { @@ -673,8 +676,9 @@ enum lttng_trace_chunk_status lttng_trace_chunk_set_as_user( status = LTTNG_TRACE_CHUNK_STATUS_ERROR; goto end; } - LTTNG_OPTIONAL_SET(&chunk->chunk_directory, - lttng_directory_handle_move(chunk_directory)); + reference_acquired = lttng_directory_handle_get(chunk_directory); + assert(reference_acquired); + chunk->chunk_directory = chunk_directory; LTTNG_OPTIONAL_SET(&chunk->mode, TRACE_CHUNK_MODE_USER); end: pthread_mutex_unlock(&chunk->lock); @@ -682,19 +686,19 @@ end: } LTTNG_HIDDEN -enum lttng_trace_chunk_status lttng_trace_chunk_get_chunk_directory_handle( +enum lttng_trace_chunk_status lttng_trace_chunk_borrow_chunk_directory_handle( struct lttng_trace_chunk *chunk, const struct lttng_directory_handle **handle) { enum lttng_trace_chunk_status status = LTTNG_TRACE_CHUNK_STATUS_OK; pthread_mutex_lock(&chunk->lock); - if (!chunk->chunk_directory.is_set) { + if (!chunk->chunk_directory) { status = LTTNG_TRACE_CHUNK_STATUS_NONE; goto end; } - *handle = &chunk->chunk_directory.value; + *handle = chunk->chunk_directory; end: pthread_mutex_unlock(&chunk->lock); return status; @@ -776,7 +780,7 @@ enum lttng_trace_chunk_status lttng_trace_chunk_create_subdirectory( status = LTTNG_TRACE_CHUNK_STATUS_INVALID_OPERATION; goto end; } - if (!chunk->chunk_directory.is_set) { + if (!chunk->chunk_directory) { ERR("Attempted to create trace chunk subdirectory \"%s\" before setting the chunk output directory", path); status = LTTNG_TRACE_CHUNK_STATUS_ERROR; @@ -789,7 +793,7 @@ enum lttng_trace_chunk_status lttng_trace_chunk_create_subdirectory( goto end; } ret = lttng_directory_handle_create_subdirectory_recursive_as_user( - &chunk->chunk_directory.value, path, + chunk->chunk_directory, path, DIR_CREATION_MODE, chunk->credentials.value.use_current_user ? NULL : &chunk->credentials.value.user); @@ -829,14 +833,14 @@ enum lttng_trace_chunk_status lttng_trace_chunk_open_file( status = LTTNG_TRACE_CHUNK_STATUS_ERROR; goto end; } - if (!chunk->chunk_directory.is_set) { + if (!chunk->chunk_directory) { ERR("Attempted to open trace chunk file \"%s\" before setting the chunk output directory", file_path); status = LTTNG_TRACE_CHUNK_STATUS_ERROR; goto end; } ret = lttng_directory_handle_open_file_as_user( - &chunk->chunk_directory.value, file_path, flags, mode, + chunk->chunk_directory, file_path, flags, mode, chunk->credentials.value.use_current_user ? NULL : &chunk->credentials.value.user); if (ret < 0) { @@ -870,14 +874,14 @@ int lttng_trace_chunk_unlink_file(struct lttng_trace_chunk *chunk, status = LTTNG_TRACE_CHUNK_STATUS_ERROR; goto end; } - if (!chunk->chunk_directory.is_set) { + if (!chunk->chunk_directory) { ERR("Attempted to unlink trace chunk file \"%s\" before setting the chunk output directory", file_path); status = LTTNG_TRACE_CHUNK_STATUS_ERROR; goto end; } ret = lttng_directory_handle_unlink_file_as_user( - &chunk->chunk_directory.value, file_path, + chunk->chunk_directory, file_path, chunk->credentials.value.use_current_user ? NULL : &chunk->credentials.value.user); if (ret < 0) { @@ -901,11 +905,11 @@ void lttng_trace_chunk_move_to_completed(struct lttng_trace_chunk *trace_chunk) LTTNG_OPTIONAL_GET(trace_chunk->timestamp_creation); const time_t close_timestamp = LTTNG_OPTIONAL_GET(trace_chunk->timestamp_close); - LTTNG_OPTIONAL(struct lttng_directory_handle) archived_chunks_directory = {}; + struct lttng_directory_handle *archived_chunks_directory = NULL; if (!trace_chunk->mode.is_set || trace_chunk->mode.value != TRACE_CHUNK_MODE_OWNER || - !trace_chunk->session_output_directory.is_set) { + !trace_chunk->session_output_directory) { /* * This command doesn't need to run if the output is remote * or if the trace chunk is not owned by this process. @@ -923,12 +927,13 @@ void lttng_trace_chunk_move_to_completed(struct lttng_trace_chunk *trace_chunk) * is renamed to match the chunk's name. */ if (chunk_id == 0) { - struct lttng_directory_handle temporary_rename_directory; + struct lttng_directory_handle *temporary_rename_directory = + NULL; size_t i, count = lttng_dynamic_pointer_array_get_count( - &trace_chunk->top_level_directories); + &trace_chunk->top_level_directories); ret = lttng_directory_handle_create_subdirectory_as_user( - &trace_chunk->session_output_directory.value, + trace_chunk->session_output_directory, DEFAULT_TEMPORARY_CHUNK_RENAME_DIRECTORY, DIR_CREATION_MODE, !trace_chunk->credentials.value.use_current_user ? @@ -938,10 +943,10 @@ void lttng_trace_chunk_move_to_completed(struct lttng_trace_chunk *trace_chunk) DEFAULT_TEMPORARY_CHUNK_RENAME_DIRECTORY); } - ret = lttng_directory_handle_init_from_handle(&temporary_rename_directory, + temporary_rename_directory = lttng_directory_handle_create_from_handle( DEFAULT_TEMPORARY_CHUNK_RENAME_DIRECTORY, - &trace_chunk->session_output_directory.value); - if (ret) { + trace_chunk->session_output_directory); + if (!temporary_rename_directory) { ERR("Failed to get handle to temporary trace chunk rename directory"); goto end; } @@ -952,9 +957,9 @@ void lttng_trace_chunk_move_to_completed(struct lttng_trace_chunk *trace_chunk) &trace_chunk->top_level_directories, i); ret = lttng_directory_handle_rename_as_user( - &trace_chunk->session_output_directory.value, + trace_chunk->session_output_directory, top_level_name, - &temporary_rename_directory, + temporary_rename_directory, top_level_name, LTTNG_OPTIONAL_GET(trace_chunk->credentials).use_current_user ? NULL : @@ -962,12 +967,12 @@ void lttng_trace_chunk_move_to_completed(struct lttng_trace_chunk *trace_chunk) if (ret) { PERROR("Failed to move \"%s\" to temporary trace chunk rename directory", top_level_name); - lttng_directory_handle_fini( - &temporary_rename_directory); + lttng_directory_handle_put( + temporary_rename_directory); goto end; } } - lttng_directory_handle_fini(&temporary_rename_directory); + lttng_directory_handle_put(temporary_rename_directory); directory_to_rename = DEFAULT_TEMPORARY_CHUNK_RENAME_DIRECTORY; free_directory_to_rename = false; } else { @@ -988,7 +993,7 @@ void lttng_trace_chunk_move_to_completed(struct lttng_trace_chunk *trace_chunk) } ret = lttng_directory_handle_create_subdirectory_as_user( - &trace_chunk->session_output_directory.value, + trace_chunk->session_output_directory, DEFAULT_ARCHIVED_TRACE_CHUNKS_DIRECTORY, DIR_CREATION_MODE, !trace_chunk->credentials.value.use_current_user ? @@ -1000,20 +1005,18 @@ void lttng_trace_chunk_move_to_completed(struct lttng_trace_chunk *trace_chunk) goto end; } - ret = lttng_directory_handle_init_from_handle( - &archived_chunks_directory.value, + archived_chunks_directory = lttng_directory_handle_create_from_handle( DEFAULT_ARCHIVED_TRACE_CHUNKS_DIRECTORY, - &trace_chunk->session_output_directory.value); - if (ret) { + trace_chunk->session_output_directory); + if (!archived_chunks_directory) { PERROR("Failed to get handle to archived trace chunks directory"); goto end; } - archived_chunks_directory.is_set = true; ret = lttng_directory_handle_rename_as_user( - &trace_chunk->session_output_directory.value, + trace_chunk->session_output_directory, directory_to_rename, - &archived_chunks_directory.value, + archived_chunks_directory, archived_chunk_name, LTTNG_OPTIONAL_GET(trace_chunk->credentials).use_current_user ? NULL : @@ -1024,9 +1027,7 @@ void lttng_trace_chunk_move_to_completed(struct lttng_trace_chunk *trace_chunk) } end: - if (archived_chunks_directory.is_set) { - lttng_directory_handle_fini(&archived_chunks_directory.value); - } + lttng_directory_handle_put(archived_chunks_directory); free(archived_chunk_name); if (free_directory_to_rename) { free(directory_to_rename); @@ -1202,15 +1203,16 @@ lttng_trace_chunk_registry_element_create_from_chunk( element->chunk = *chunk; lttng_trace_chunk_init(&element->chunk); - if (chunk->session_output_directory.is_set) { - element->chunk.session_output_directory.value = - lttng_directory_handle_move( - &chunk->session_output_directory.value); - } - if (chunk->chunk_directory.is_set) { - element->chunk.chunk_directory.value = - lttng_directory_handle_move( - &chunk->chunk_directory.value); + if (chunk->session_output_directory) { + /* Transferred ownership. */ + element->chunk.session_output_directory = + chunk->session_output_directory; + chunk->session_output_directory = NULL; + } + if (chunk->chunk_directory) { + /* Transferred ownership. */ + element->chunk.chunk_directory = chunk->chunk_directory; + chunk->chunk_directory = NULL; } /* * The original chunk becomes invalid; the name attribute is transferred diff --git a/src/common/trace-chunk.h b/src/common/trace-chunk.h index 41c50b637..fcacb01ff 100644 --- a/src/common/trace-chunk.h +++ b/src/common/trace-chunk.h @@ -131,20 +131,18 @@ LTTNG_HIDDEN enum lttng_trace_chunk_status lttng_trace_chunk_set_credentials_current_user( struct lttng_trace_chunk *chunk); -/* session_output_directory ownership is transferred to the chunk on success. */ LTTNG_HIDDEN enum lttng_trace_chunk_status lttng_trace_chunk_set_as_owner( struct lttng_trace_chunk *chunk, struct lttng_directory_handle *session_output_directory); -/* chunk_output_directory ownership is transferred to the chunk on success. */ LTTNG_HIDDEN enum lttng_trace_chunk_status lttng_trace_chunk_set_as_user( struct lttng_trace_chunk *chunk, struct lttng_directory_handle *chunk_directory); LTTNG_HIDDEN -enum lttng_trace_chunk_status lttng_trace_chunk_get_chunk_directory_handle( +enum lttng_trace_chunk_status lttng_trace_chunk_borrow_chunk_directory_handle( struct lttng_trace_chunk *chunk, const struct lttng_directory_handle **handle); diff --git a/src/common/ust-consumer/ust-consumer.c b/src/common/ust-consumer/ust-consumer.c index 27f503405..32dfd5bb4 100644 --- a/src/common/ust-consumer/ust-consumer.c +++ b/src/common/ust-consumer/ust-consumer.c @@ -2039,8 +2039,7 @@ end_rotate_channel_nosignal: *msg.u.create_trace_chunk.override_name ? msg.u.create_trace_chunk.override_name : NULL; - LTTNG_OPTIONAL(struct lttng_directory_handle) chunk_directory_handle = - LTTNG_OPTIONAL_INIT; + struct lttng_directory_handle *chunk_directory_handle = NULL; /* * The session daemon will only provide a chunk directory file @@ -2065,17 +2064,15 @@ end_rotate_channel_nosignal: DBG("Received trace chunk directory fd (%d)", chunk_dirfd); - ret = lttng_directory_handle_init_from_dirfd( - &chunk_directory_handle.value, + chunk_directory_handle = lttng_directory_handle_create_from_dirfd( chunk_dirfd); - if (ret) { + if (!chunk_directory_handle) { ERR("Failed to initialize chunk directory handle from directory file descriptor"); if (close(chunk_dirfd)) { PERROR("Failed to close chunk directory file descriptor"); } goto error_fatal; } - chunk_directory_handle.is_set = true; } ret_code = lttng_consumer_create_trace_chunk( @@ -2088,14 +2085,8 @@ end_rotate_channel_nosignal: msg.u.create_trace_chunk.credentials.is_set ? &credentials : NULL, - chunk_directory_handle.is_set ? - &chunk_directory_handle.value : - NULL); - - if (chunk_directory_handle.is_set) { - lttng_directory_handle_fini( - &chunk_directory_handle.value); - } + chunk_directory_handle); + lttng_directory_handle_put(chunk_directory_handle); goto end_msg_sessiond; } case LTTNG_CONSUMER_CLOSE_TRACE_CHUNK: diff --git a/src/common/utils.c b/src/common/utils.c index 2e07d4ff0..f3cb9e030 100644 --- a/src/common/utils.c +++ b/src/common/utils.c @@ -682,21 +682,22 @@ LTTNG_HIDDEN int utils_mkdir(const char *path, mode_t mode, int uid, int gid) { int ret; - struct lttng_directory_handle handle; + struct lttng_directory_handle *handle; const struct lttng_credentials creds = { .uid = (uid_t) uid, .gid = (gid_t) gid, }; - ret = lttng_directory_handle_init(&handle, NULL); - if (ret) { + handle = lttng_directory_handle_create(NULL); + if (!handle) { + ret = -1; goto end; } ret = lttng_directory_handle_create_subdirectory_as_user( - &handle, path, mode, + handle, path, mode, (uid >= 0 || gid >= 0) ? &creds : NULL); - lttng_directory_handle_fini(&handle); end: + lttng_directory_handle_put(handle); return ret; } @@ -710,21 +711,22 @@ LTTNG_HIDDEN int utils_mkdir_recursive(const char *path, mode_t mode, int uid, int gid) { int ret; - struct lttng_directory_handle handle; + struct lttng_directory_handle *handle; const struct lttng_credentials creds = { .uid = (uid_t) uid, .gid = (gid_t) gid, }; - ret = lttng_directory_handle_init(&handle, NULL); - if (ret) { + handle = lttng_directory_handle_create(NULL); + if (!handle) { + ret = -1; goto end; } ret = lttng_directory_handle_create_subdirectory_recursive_as_user( - &handle, path, mode, + handle, path, mode, (uid >= 0 || gid >= 0) ? &creds : NULL); - lttng_directory_handle_fini(&handle); end: + lttng_directory_handle_put(handle); return ret; } @@ -1353,15 +1355,16 @@ LTTNG_HIDDEN int utils_recursive_rmdir(const char *path) { int ret; - struct lttng_directory_handle handle; + struct lttng_directory_handle *handle; - ret = lttng_directory_handle_init(&handle, NULL); - if (ret) { + handle = lttng_directory_handle_create(NULL); + if (!handle) { + ret = -1; goto end; } - ret = lttng_directory_handle_remove_subdirectory(&handle, path); - lttng_directory_handle_fini(&handle); + ret = lttng_directory_handle_remove_subdirectory(handle, path); end: + lttng_directory_handle_put(handle); return ret; } diff --git a/tests/unit/test_directory_handle.c b/tests/unit/test_directory_handle.c index da77c43d5..c14066909 100644 --- a/tests/unit/test_directory_handle.c +++ b/tests/unit/test_directory_handle.c @@ -162,40 +162,41 @@ end: static int test_rmdir_fail_non_empty(const char *test_dir) { int ret, tests_ran = 0; - struct lttng_directory_handle test_dir_handle; + struct lttng_directory_handle *test_dir_handle; char *created_dir = NULL; const char test_root_name[] = "fail_non_empty"; char *test_dir_path = NULL; diag("rmdir (fail if non-empty)"); - ret = lttng_directory_handle_init(&test_dir_handle, test_dir); - ok(ret == 0, "Initialized directory handle from the test directory"); + test_dir_handle = lttng_directory_handle_create(test_dir); + ok(test_dir_handle, "Initialized directory handle from the test directory"); tests_ran++; - if (ret) { + if (!test_dir_handle) { + ret = -1; goto end; } - ret = create_non_empty_hierarchy_with_root(&test_dir_handle, test_root_name); + ret = create_non_empty_hierarchy_with_root(test_dir_handle, test_root_name); if (ret) { diag("Failed to setup folder/file hierarchy to run test"); goto end; } ret = lttng_directory_handle_remove_subdirectory_recursive( - &test_dir_handle, test_root_name, + test_dir_handle, test_root_name, LTTNG_DIRECTORY_HANDLE_FAIL_NON_EMPTY_FLAG); ok(ret == -1, "Error returned when attempting to recursively remove non-empty hierarchy with LTTNG_DIRECTORY_HANDLE_FAIL_NON_EMPTY_FLAG"); tests_ran++; - ret = remove_file_from_hierarchy(&test_dir_handle, test_root_name); + ret = remove_file_from_hierarchy(test_dir_handle, test_root_name); if (ret) { diag("Failed to remove file from test folder hierarchy"); goto end; } ret = lttng_directory_handle_remove_subdirectory_recursive( - &test_dir_handle, test_root_name, + test_dir_handle, test_root_name, LTTNG_DIRECTORY_HANDLE_FAIL_NON_EMPTY_FLAG); ok(ret == 0, "No error returned when recursively removing empty hierarchy with LTTNG_DIRECTORY_HANDLE_FAIL_NON_EMPTY_FLAG"); tests_ran++; @@ -211,7 +212,7 @@ static int test_rmdir_fail_non_empty(const char *test_dir) tests_ran++; ret = 0; end: - lttng_directory_handle_fini(&test_dir_handle); + lttng_directory_handle_put(test_dir_handle); free(created_dir); free(test_dir_path); return ret == 0 ? tests_ran : ret; @@ -220,28 +221,29 @@ end: static int test_rmdir_skip_non_empty(const char *test_dir) { int ret, tests_ran = 0; - struct lttng_directory_handle test_dir_handle; + struct lttng_directory_handle *test_dir_handle; char *created_dir = NULL; const char test_root_name[] = "skip_non_empty"; char *test_dir_path = NULL; diag("rmdir (skip if non-empty)"); - ret = lttng_directory_handle_init(&test_dir_handle, test_dir); - ok(ret == 0, "Initialized directory handle from the test directory"); + test_dir_handle = lttng_directory_handle_create(test_dir); + ok(test_dir_handle, "Initialized directory handle from the test directory"); tests_ran++; - if (ret) { + if (!test_dir_handle) { + ret = -1; goto end; } - ret = create_non_empty_hierarchy_with_root(&test_dir_handle, test_root_name); + ret = create_non_empty_hierarchy_with_root(test_dir_handle, test_root_name); if (ret) { diag("Failed to setup folder/file hierarchy to run test"); goto end; } ret = lttng_directory_handle_remove_subdirectory_recursive( - &test_dir_handle, test_root_name, + test_dir_handle, test_root_name, LTTNG_DIRECTORY_HANDLE_SKIP_NON_EMPTY_FLAG); ok(ret == 0, "No error returned when attempting to recursively remove non-empty hierarchy with LTTNG_DIRECTORY_HANDLE_SKIP_NON_EMPTY_FLAG"); tests_ran++; @@ -254,14 +256,14 @@ static int test_rmdir_skip_non_empty(const char *test_dir) ok(dir_exists(test_dir_path), "Test directory still exists after skip"); tests_ran++; - ret = remove_file_from_hierarchy(&test_dir_handle, test_root_name); + ret = remove_file_from_hierarchy(test_dir_handle, test_root_name); if (ret) { diag("Failed to remove file from test folder hierarchy"); goto end; } ret = lttng_directory_handle_remove_subdirectory_recursive( - &test_dir_handle, test_root_name, + test_dir_handle, test_root_name, LTTNG_DIRECTORY_HANDLE_SKIP_NON_EMPTY_FLAG); ok(ret == 0, "No error returned when recursively removing empty hierarchy with LTTNG_DIRECTORY_HANDLE_SKIP_NON_EMPTY_FLAG"); tests_ran++; @@ -272,7 +274,7 @@ static int test_rmdir_skip_non_empty(const char *test_dir) tests_ran++; ret = 0; end: - lttng_directory_handle_fini(&test_dir_handle); + lttng_directory_handle_put(test_dir_handle); free(created_dir); free(test_dir_path); return ret == 0 ? tests_ran : ret;