From 6cd525e813795a1d5e38feac8dedf2c73ffb1274 Mon Sep 17 00:00:00 2001 From: Mathieu Desnoyers Date: Mon, 11 Nov 2013 14:12:26 -0500 Subject: [PATCH] Use lttng_read/lttng_write wrappers This ensures we deal with EINTR and partial reads in the same way everywhere. This also fixes non-null-terminated string bug in src/common/compat/compat-epoll.c and src/common/sessiond-comm/inet.c. Also, lttng_pipe_read() and lttng_pipe_write() return values are changed to simply match those of lttng_read() and lttng_write(). Their implementation is replaced by the wrappers, except for locking and checks. Signed-off-by: Mathieu Desnoyers --- src/bin/lttng-relayd/live.c | 50 ++++++---------- src/bin/lttng-relayd/main.c | 59 ++++++++----------- src/bin/lttng-sessiond/ht-cleanup.c | 10 ++-- src/bin/lttng-sessiond/main.c | 23 ++++---- src/bin/lttng-sessiond/ust-thread.c | 10 ++-- src/bin/lttng-sessiond/utils.c | 18 +++--- src/common/compat/compat-epoll.c | 10 +++- src/common/consumer-metadata-cache.c | 6 +- src/common/consumer-stream.c | 9 ++- src/common/consumer.c | 69 +++++++++++----------- src/common/index/index.c | 21 ++++--- src/common/index/index.h | 2 +- src/common/pipe.c | 80 ++++---------------------- src/common/readwrite.c | 6 +- src/common/runas.c | 69 ++++++++-------------- src/common/sessiond-comm/inet.c | 10 +++- src/common/ust-consumer/ust-consumer.c | 12 ++-- 17 files changed, 187 insertions(+), 277 deletions(-) diff --git a/src/bin/lttng-relayd/live.c b/src/bin/lttng-relayd/live.c index 5052eae47..b0cc63128 100644 --- a/src/bin/lttng-relayd/live.c +++ b/src/bin/lttng-relayd/live.c @@ -106,16 +106,14 @@ void cleanup(void) static int notify_thread_pipe(int wpipe) { - int ret; + ssize_t ret; - do { - ret = write(wpipe, "!", 1); - } while (ret < 0 && errno == EINTR); - if (ret < 0 || ret != 1) { + ret = lttng_write(wpipe, "!", 1); + if (ret < 1) { PERROR("write poll pipe"); } - return ret; + return (int) ret; } /* @@ -374,7 +372,8 @@ error_sock_control: static void *thread_dispatcher(void *data) { - int ret, err = -1; + int err = -1; + ssize_t ret; struct cds_wfq_node *node; struct relay_command *relay_cmd = NULL; @@ -411,12 +410,10 @@ void *thread_dispatcher(void *data) * so we can be assured that the data will be read at some point in * time or wait to the end of the world :) */ - do { - ret = write(live_relay_cmd_pipe[1], relay_cmd, - sizeof(*relay_cmd)); - } while (ret < 0 && errno == EINTR); + ret = lttng_write(live_relay_cmd_pipe[1], relay_cmd, + sizeof(*relay_cmd)); free(relay_cmd); - if (ret < 0 || ret != sizeof(struct relay_command)) { + if (ret < sizeof(struct relay_command)) { PERROR("write cmd pipe"); goto error; } @@ -647,11 +644,8 @@ static int open_index(struct relay_viewer_stream *stream) stream->index_read_fd = ret; DBG("Opening index file %s in read only, (fd: %d)", fullpath, ret); - do { - health_code_update(); - ret = read(stream->index_read_fd, &hdr, sizeof(hdr)); - } while (ret < 0 && errno == EINTR); - if (ret < 0) { + ret = lttng_read(stream->index_read_fd, &hdr, sizeof(hdr)); + if (ret < sizeof(hdr)) { PERROR("Reading index header"); goto error; } @@ -1064,11 +1058,8 @@ int viewer_get_next_index(struct relay_command *cmd, viewer_index.flags |= LTTNG_VIEWER_FLAG_NEW_METADATA; } - do { - health_code_update(); - ret = read(vstream->index_read_fd, &packet_index, - sizeof(packet_index)); - } while (ret < 0 && errno == EINTR); + ret = lttng_read(vstream->index_read_fd, &packet_index, + sizeof(packet_index)); if (ret < sizeof(packet_index)) { PERROR("Relay reading index file"); viewer_index.status = htobe32(VIEWER_INDEX_ERR); @@ -1197,8 +1188,8 @@ int viewer_get_packet(struct relay_command *cmd) PERROR("lseek"); goto error; } - read_len = read(stream->read_fd, data, len); - if (read_len < (ssize_t) len) { + read_len = lttng_read(stream->read_fd, data, len); + if (read_len < len) { PERROR("Relay reading trace file, fd: %d, offset: %" PRIu64, stream->read_fd, be64toh(get_packet_info.offset)); goto error; @@ -1320,8 +1311,8 @@ int viewer_get_metadata(struct relay_command *cmd) goto error; } - read_len = read(stream->read_fd, data, len); - if (read_len < (ssize_t) len) { + read_len = lttng_read(stream->read_fd, data, len); + if (read_len < len) { PERROR("Relay reading metadata file"); goto error; } @@ -1453,11 +1444,8 @@ int add_connection(int fd, struct lttng_poll_event *events, goto error; } - do { - health_code_update(); - ret = read(fd, relay_connection, sizeof(*relay_connection)); - } while (ret < 0 && errno == EINTR); - if (ret < 0 || ret < sizeof(*relay_connection)) { + ret = lttng_read(fd, relay_connection, sizeof(*relay_connection)); + if (ret < sizeof(*relay_connection)) { PERROR("read relay cmd pipe"); goto error_read; } diff --git a/src/bin/lttng-relayd/main.c b/src/bin/lttng-relayd/main.c index 1bdef652e..0005714c5 100644 --- a/src/bin/lttng-relayd/main.c +++ b/src/bin/lttng-relayd/main.c @@ -293,12 +293,10 @@ void cleanup(void) static int notify_thread_pipe(int wpipe) { - int ret; + ssize_t ret; - do { - ret = write(wpipe, "!", 1); - } while (ret < 0 && errno == EINTR); - if (ret < 0 || ret != 1) { + ret = lttng_write(wpipe, "!", 1); + if (ret < 1) { PERROR("write poll pipe"); } @@ -307,12 +305,10 @@ int notify_thread_pipe(int wpipe) static void notify_health_quit_pipe(int *pipe) { - int ret; + ssize_t ret; - do { - ret = write(pipe[1], "4", 1); - } while (ret < 0 && errno == EINTR); - if (ret < 0 || ret != 1) { + ret = lttng_write(pipe[1], "4", 1); + if (ret < 1) { PERROR("write relay health quit"); } } @@ -707,7 +703,8 @@ error_sock_control: static void *relay_thread_dispatcher(void *data) { - int ret, err = -1; + int err = -1; + ssize_t ret; struct cds_wfq_node *node; struct relay_command *relay_cmd = NULL; @@ -742,12 +739,10 @@ void *relay_thread_dispatcher(void *data) * call is blocking so we can be assured that the data will be read * at some point in time or wait to the end of the world :) */ - do { - ret = write(relay_cmd_pipe[1], relay_cmd, - sizeof(struct relay_command)); - } while (ret < 0 && errno == EINTR); + ret = lttng_write(relay_cmd_pipe[1], relay_cmd, + sizeof(struct relay_command)); free(relay_cmd); - if (ret < 0 || ret != sizeof(struct relay_command)) { + if (ret < sizeof(struct relay_command)) { PERROR("write cmd pipe"); goto error; } @@ -1248,7 +1243,7 @@ int relay_start(struct lttcomm_relayd_hdr *recv_hdr, */ static int write_padding_to_file(int fd, uint32_t size) { - int ret = 0; + ssize_t ret = 0; char *zeros; if (size == 0) { @@ -1262,10 +1257,8 @@ static int write_padding_to_file(int fd, uint32_t size) goto end; } - do { - ret = write(fd, zeros, size); - } while (ret < 0 && errno == EINTR); - if (ret < 0 || ret != size) { + ret = lttng_write(fd, zeros, size); + if (ret < size) { PERROR("write padding to file"); } @@ -1283,6 +1276,7 @@ int relay_recv_metadata(struct lttcomm_relayd_hdr *recv_hdr, struct relay_command *cmd) { int ret = htobe32(LTTNG_OK); + ssize_t size_ret; struct relay_session *session = cmd->session; struct lttcomm_relayd_metadata_payload *metadata_struct; struct relay_stream *metadata_stream; @@ -1339,11 +1333,9 @@ int relay_recv_metadata(struct lttcomm_relayd_hdr *recv_hdr, goto end_unlock; } - do { - ret = write(metadata_stream->fd, metadata_struct->payload, - payload_size); - } while (ret < 0 && errno == EINTR); - if (ret < 0 || ret != payload_size) { + size_ret = lttng_write(metadata_stream->fd, metadata_struct->payload, + payload_size); + if (size_ret < payload_size) { ERR("Relay error writing metadata on file"); ret = -1; goto end_unlock; @@ -1984,6 +1976,7 @@ static int relay_process_data(struct relay_command *cmd) { int ret = 0, rotate_index = 0; + ssize_t size_ret; struct relay_stream *stream; struct lttcomm_relayd_data_hdr data_hdr; uint64_t stream_id; @@ -2071,10 +2064,8 @@ int relay_process_data(struct relay_command *cmd) } /* Write data to stream output fd. */ - do { - ret = write(stream->fd, data_buffer, data_size); - } while (ret < 0 && errno == EINTR); - if (ret < 0 || ret != data_size) { + size_ret = lttng_write(stream->fd, data_buffer, data_size); + if (size_ret < data_size) { ERR("Relay error writing data to file"); ret = -1; goto end_rcu_unlock; @@ -2120,17 +2111,15 @@ int relay_add_connection(int fd, struct lttng_poll_event *events, struct lttng_ht *relay_connections_ht) { struct relay_command *relay_connection; - int ret; + ssize_t ret; relay_connection = zmalloc(sizeof(struct relay_command)); if (relay_connection == NULL) { PERROR("Relay command zmalloc"); goto error; } - do { - ret = read(fd, relay_connection, sizeof(struct relay_command)); - } while (ret < 0 && errno == EINTR); - if (ret < 0 || ret < sizeof(struct relay_command)) { + ret = lttng_read(fd, relay_connection, sizeof(struct relay_command)); + if (ret < sizeof(struct relay_command)) { PERROR("read relay cmd pipe"); goto error_read; } diff --git a/src/bin/lttng-sessiond/ht-cleanup.c b/src/bin/lttng-sessiond/ht-cleanup.c index 890c9a821..48b0be6e4 100644 --- a/src/bin/lttng-sessiond/ht-cleanup.c +++ b/src/bin/lttng-sessiond/ht-cleanup.c @@ -28,6 +28,7 @@ void *thread_ht_cleanup(void *data) { int ret, i, pollfd, err = -1; + ssize_t size_ret; uint32_t revents, nb_fd; struct lttng_poll_event events; @@ -100,11 +101,10 @@ restart: goto error; } - do { - /* Get socket from dispatch thread. */ - ret = read(ht_cleanup_pipe[0], &ht, sizeof(ht)); - } while (ret < 0 && errno == EINTR); - if (ret < 0 || ret < sizeof(ht)) { + /* Get socket from dispatch thread. */ + size_ret = lttng_read(ht_cleanup_pipe[0], &ht, + sizeof(ht)); + if (size_ret < sizeof(ht)) { PERROR("ht cleanup notify pipe"); goto error; } diff --git a/src/bin/lttng-sessiond/main.c b/src/bin/lttng-sessiond/main.c index 54b47567d..a81bca695 100644 --- a/src/bin/lttng-sessiond/main.c +++ b/src/bin/lttng-sessiond/main.c @@ -782,6 +782,7 @@ static void update_ust_app(int app_sock) static void *thread_manage_kernel(void *data) { int ret, i, pollfd, update_poll_flag = 1, err = -1; + ssize_t size_ret; uint32_t revents, nb_fd; char tmp; struct lttng_poll_event events; @@ -871,9 +872,8 @@ static void *thread_manage_kernel(void *data) /* Check for data on kernel pipe */ if (pollfd == kernel_poll_pipe[0] && (revents & LPOLLIN)) { - do { - ret = read(kernel_poll_pipe[0], &tmp, 1); - } while (ret < 0 && errno == EINTR); + (void) lttng_read(kernel_poll_pipe[0], + &tmp, 1); /* * Ret value is useless here, if this pipe gets any actions an * update is required anyway. @@ -1245,6 +1245,7 @@ error_poll: static void *thread_manage_apps(void *data) { int i, ret, pollfd, err = -1; + ssize_t size_ret; uint32_t revents, nb_fd; struct lttng_poll_event events; @@ -1320,10 +1321,8 @@ static void *thread_manage_apps(void *data) int sock; /* Empty pipe */ - do { - ret = read(apps_cmd_pipe[0], &sock, sizeof(sock)); - } while (ret < 0 && errno == EINTR); - if (ret < 0 || ret < sizeof(sock)) { + size_ret = lttng_read(apps_cmd_pipe[0], &sock, sizeof(sock)); + if (size_ret < sizeof(sock)) { PERROR("read apps cmd pipe"); goto error; } @@ -1406,7 +1405,7 @@ error_testpoint: */ static int send_socket_to_thread(int fd, int sock) { - int ret; + ssize_t ret; /* * It's possible that the FD is set as invalid with -1 concurrently just @@ -1417,10 +1416,8 @@ static int send_socket_to_thread(int fd, int sock) goto error; } - do { - ret = write(fd, &sock, sizeof(sock)); - } while (ret < 0 && errno == EINTR); - if (ret < 0 || ret != sizeof(sock)) { + ret = lttng_write(fd, &sock, sizeof(sock)); + if (ret < sizeof(sock)) { PERROR("write apps pipe %d", fd); if (ret < 0) { ret = -errno; @@ -1431,7 +1428,7 @@ static int send_socket_to_thread(int fd, int sock) /* All good. Don't send back the write positive ret value. */ ret = 0; error: - return ret; + return (int) ret; } /* diff --git a/src/bin/lttng-sessiond/ust-thread.c b/src/bin/lttng-sessiond/ust-thread.c index 85803e472..0248d7a0a 100644 --- a/src/bin/lttng-sessiond/ust-thread.c +++ b/src/bin/lttng-sessiond/ust-thread.c @@ -31,6 +31,7 @@ void *ust_thread_manage_notify(void *data) { int i, ret, pollfd, err = -1; + ssize_t size_ret; uint32_t revents, nb_fd; struct lttng_poll_event events; @@ -105,11 +106,10 @@ restart: goto error; } - do { - /* Get socket from dispatch thread. */ - ret = read(apps_cmd_notify_pipe[0], &sock, sizeof(sock)); - } while (ret < 0 && errno == EINTR); - if (ret < 0 || ret < sizeof(sock)) { + /* Get socket from dispatch thread. */ + size_ret = lttng_read(apps_cmd_notify_pipe[0], + &sock, sizeof(sock)); + if (size_ret < sizeof(sock)) { PERROR("read apps notify pipe"); goto error; } diff --git a/src/bin/lttng-sessiond/utils.c b/src/bin/lttng-sessiond/utils.c index 85a1f0203..2ff57cd14 100644 --- a/src/bin/lttng-sessiond/utils.c +++ b/src/bin/lttng-sessiond/utils.c @@ -32,34 +32,30 @@ int ht_cleanup_pipe[2] = { -1, -1 }; */ int notify_thread_pipe(int wpipe) { - int ret; + ssize_t ret; /* Ignore if the pipe is invalid. */ if (wpipe < 0) { return 0; } - do { - ret = write(wpipe, "!", 1); - } while (ret < 0 && errno == EINTR); - if (ret < 0) { + ret = lttng_write(wpipe, "!", 1); + if (ret < 1) { PERROR("write poll pipe"); } - return ret; + return (int) ret; } void ht_cleanup_push(struct lttng_ht *ht) { - int ret; + ssize_t ret; int fd = ht_cleanup_pipe[1]; if (fd < 0) return; - do { - ret = write(fd, &ht, sizeof(ht)); - } while (ret < 0 && errno == EINTR); - if (ret < 0 || ret != sizeof(ht)) { + ret = lttng_write(fd, &ht, sizeof(ht)); + if (ret < sizeof(ht)) { PERROR("write ht cleanup pipe %d", fd); if (ret < 0) { ret = -errno; diff --git a/src/common/compat/compat-epoll.c b/src/common/compat/compat-epoll.c index 903a9f825..1380ce855 100644 --- a/src/common/compat/compat-epoll.c +++ b/src/common/compat/compat-epoll.c @@ -250,6 +250,7 @@ error: void compat_epoll_set_max_size(void) { int ret, fd; + size_t size_ret; char buf[64]; poll_max_size = DEFAULT_POLL_SIZE; @@ -259,11 +260,16 @@ void compat_epoll_set_max_size(void) return; } - ret = read(fd, buf, sizeof(buf)); - if (ret < 0) { + size_ret = lttng_read(fd, buf, sizeof(buf)); + /* + * Allow reading a file smaller than buf, but keep space for + * final \0. + */ + if (size_ret < 0 || size_ret >= sizeof(buf)) { PERROR("read set max size"); goto error; } + buf[size_ret] = '\0'; poll_max_size = atoi(buf); if (poll_max_size == 0) { diff --git a/src/common/consumer-metadata-cache.c b/src/common/consumer-metadata-cache.c index d597e64e3..d8f5001f8 100644 --- a/src/common/consumer-metadata-cache.c +++ b/src/common/consumer-metadata-cache.c @@ -81,6 +81,7 @@ int consumer_metadata_cache_write(struct lttng_consumer_channel *channel, unsigned int offset, unsigned int len, char *data) { int ret = 0; + int size_ret; struct consumer_metadata_cache *cache; assert(channel); @@ -109,10 +110,11 @@ int consumer_metadata_cache_write(struct lttng_consumer_channel *channel, cache->contiguous = cache->max_offset; if (channel->monitor) { - ret = write(channel->metadata_stream->ust_metadata_poll_pipe[1], + size_ret = lttng_write(channel->metadata_stream->ust_metadata_poll_pipe[1], &dummy, 1); - if (ret < 1) { + if (size_ret < 1) { ERR("Wakeup UST metadata pipe"); + ret = -1; goto end; } } diff --git a/src/common/consumer-stream.c b/src/common/consumer-stream.c index 808cae236..063ba50ab 100644 --- a/src/common/consumer-stream.c +++ b/src/common/consumer-stream.c @@ -345,8 +345,15 @@ int consumer_stream_write_index(struct lttng_consumer_stream *stream, ret = relayd_send_index(&relayd->control_sock, index, stream->relayd_stream_id, stream->next_net_seq_num - 1); } else { - ret = index_write(stream->index_fd, index, + ssize_t size_ret; + + size_ret = index_write(stream->index_fd, index, sizeof(struct lttng_packet_index)); + if (size_ret < sizeof(struct lttng_packet_index)) { + ret = -1; + } else { + ret = 0; + } } if (ret < 0) { goto error; diff --git a/src/common/consumer.c b/src/common/consumer.c index 187c0c30c..8aa890349 100644 --- a/src/common/consumer.c +++ b/src/common/consumer.c @@ -96,12 +96,10 @@ static void notify_thread_lttng_pipe(struct lttng_pipe *pipe) static void notify_health_quit_pipe(int *pipe) { - int ret; + ssize_t ret; - do { - ret = write(pipe[1], "4", 1); - } while (ret < 0 && errno == EINTR); - if (ret < 0 || ret != 1) { + ret = lttng_write(pipe[1], "4", 1); + if (ret < 1) { PERROR("write consumer health quit"); } } @@ -112,16 +110,17 @@ static void notify_channel_pipe(struct lttng_consumer_local_data *ctx, enum consumer_channel_action action) { struct consumer_channel_msg msg; - int ret; + ssize_t ret; memset(&msg, 0, sizeof(msg)); msg.action = action; msg.chan = chan; msg.key = key; - do { - ret = write(ctx->consumer_channel_pipe[1], &msg, sizeof(msg)); - } while (ret < 0 && errno == EINTR); + ret = lttng_write(ctx->consumer_channel_pipe[1], &msg, sizeof(msg)); + if (ret < sizeof(msg)) { + PERROR("notify_channel_pipe write error"); + } } void notify_thread_del_channel(struct lttng_consumer_local_data *ctx, @@ -136,17 +135,18 @@ static int read_channel_pipe(struct lttng_consumer_local_data *ctx, enum consumer_channel_action *action) { struct consumer_channel_msg msg; - int ret; + ssize_t ret; - do { - ret = read(ctx->consumer_channel_pipe[0], &msg, sizeof(msg)); - } while (ret < 0 && errno == EINTR); - if (ret > 0) { - *action = msg.action; - *chan = msg.chan; - *key = msg.key; + ret = lttng_read(ctx->consumer_channel_pipe[0], &msg, sizeof(msg)); + if (ret < sizeof(msg)) { + ret = -1; + goto error; } - return ret; + *action = msg.action; + *chan = msg.chan; + *key = msg.key; +error: + return (int) ret; } /* @@ -1117,12 +1117,11 @@ void lttng_consumer_cleanup(void) */ void lttng_consumer_should_exit(struct lttng_consumer_local_data *ctx) { - int ret; + ssize_t ret; + consumer_quit = 1; - do { - ret = write(ctx->consumer_should_quit[1], "4", 1); - } while (ret < 0 && errno == EINTR); - if (ret < 0 || ret != 1) { + ret = lttng_write(ctx->consumer_should_quit[1], "4", 1); + if (ret < 1) { PERROR("write consumer quit"); } @@ -1296,15 +1295,13 @@ static int write_relayd_metadata_id(int fd, struct lttng_consumer_stream *stream, struct consumer_relayd_sock_pair *relayd, unsigned long padding) { - int ret; + ssize_t ret; struct lttcomm_relayd_metadata_payload hdr; hdr.stream_id = htobe64(stream->relayd_stream_id); hdr.padding_size = htobe32(padding); - do { - ret = write(fd, (void *) &hdr, sizeof(hdr)); - } while (ret < 0 && errno == EINTR); - if (ret < 0 || ret != sizeof(hdr)) { + ret = lttng_write(fd, (void *) &hdr, sizeof(hdr)); + if (ret < sizeof(hdr)) { /* * This error means that the fd's end is closed so ignore the perror * not to clubber the error output since this can happen in a normal @@ -1326,7 +1323,7 @@ static int write_relayd_metadata_id(int fd, stream->relayd_stream_id, padding); end: - return ret; + return (int) ret; } /* @@ -1482,11 +1479,9 @@ ssize_t lttng_consumer_on_read_subbuffer_mmap( } while (len > 0) { - do { - ret = write(outfd, mmap_base + mmap_offset, len); - } while (ret < 0 && errno == EINTR); + ret = lttng_write(outfd, mmap_base + mmap_offset, len); DBG("Consumer mmap write() ret %zd (len %lu)", ret, len); - if (ret < 0) { + if (ret < len) { /* * This is possible if the fd is closed on the other side (outfd) * or any write problem. It can be verbose a bit for a normal @@ -2282,8 +2277,8 @@ restart: pipe_len = lttng_pipe_read(ctx->consumer_metadata_pipe, &stream, sizeof(stream)); - if (pipe_len < 0) { - ERR("read metadata stream, ret: %zd", pipe_len); + if (pipe_len < sizeof(stream)) { + PERROR("read metadata stream"); /* * Continue here to handle the rest of the streams. */ @@ -2517,8 +2512,8 @@ void *consumer_thread_data_poll(void *data) DBG("consumer_data_pipe wake up"); pipe_readlen = lttng_pipe_read(ctx->consumer_data_pipe, &new_stream, sizeof(new_stream)); - if (pipe_readlen < 0) { - ERR("Consumer data pipe ret %zd", pipe_readlen); + if (pipe_readlen < sizeof(new_stream)) { + PERROR("Consumer data pipe"); /* Continue so we can at least handle the current stream(s). */ continue; } diff --git a/src/common/index/index.c b/src/common/index/index.c index 89b4fd769..cddc58c09 100644 --- a/src/common/index/index.c +++ b/src/common/index/index.c @@ -35,6 +35,7 @@ int index_create_file(char *path_name, char *stream_name, int uid, int gid, uint64_t size, uint64_t count) { int ret, fd = -1; + ssize_t size_ret; struct lttng_packet_index_file_hdr hdr; char fullpath[PATH_MAX]; @@ -65,11 +66,10 @@ int index_create_file(char *path_name, char *stream_name, int uid, int gid, hdr.index_major = htobe32(INDEX_MAJOR); hdr.index_minor = htobe32(INDEX_MINOR); - do { - ret = write(fd, &hdr, sizeof(hdr)); - } while (ret < 0 && errno == EINTR); - if (ret < 0) { + size_ret = lttng_write(fd, &hdr, sizeof(hdr)); + if (size_ret < sizeof(hdr)) { PERROR("write index header"); + ret = -1; goto error; } @@ -90,19 +90,18 @@ error: /* * Write index values to the given fd of size len. * - * Return 0 on success or else a negative value on error. + * Return "len" on success or else < len on error. errno contains error + * details. */ -int index_write(int fd, struct lttng_packet_index *index, size_t len) +ssize_t index_write(int fd, struct lttng_packet_index *index, size_t len) { - int ret; + ssize_t ret; assert(fd >= 0); assert(index); - do { - ret = write(fd, index, len); - } while (ret < 0 && errno == EINTR); - if (ret < 0) { + ret = lttng_write(fd, index, len); + if (ret < len) { PERROR("writing index file"); } diff --git a/src/common/index/index.h b/src/common/index/index.h index 58f2ac70a..4ffc52659 100644 --- a/src/common/index/index.h +++ b/src/common/index/index.h @@ -25,6 +25,6 @@ int index_create_file(char *path_name, char *stream_name, int uid, int gid, uint64_t size, uint64_t count); -int index_write(int fd, struct lttng_packet_index *index, size_t len); +ssize_t index_write(int fd, struct lttng_packet_index *index, size_t len); #endif /* _INDEX_H */ diff --git a/src/common/pipe.c b/src/common/pipe.c index 713db9739..5e2fc6c39 100644 --- a/src/common/pipe.c +++ b/src/common/pipe.c @@ -257,51 +257,23 @@ void lttng_pipe_destroy(struct lttng_pipe *pipe) /* * Read on a lttng pipe and put the data in buf of at least size count. * - * Return 0 on success or else a negative errno message from read(2). + * Return "count" on success. Return < count on error. errno can be used + * to check the actual error. */ ssize_t lttng_pipe_read(struct lttng_pipe *pipe, void *buf, size_t count) { - ssize_t ret, read_len, read_left, index; + ssize_t ret; assert(pipe); assert(buf); lock_read_side(pipe); - if (!lttng_pipe_is_read_open(pipe)) { - ret = -EBADF; + ret = -1; + errno = EBADF; goto error; } - - read_left = count; - index = 0; - do { - read_len = read(pipe->fd[0], buf + index, read_left); - if (read_len < 0) { - ret = -errno; - if (errno == EINTR) { - /* Read again. */ - continue; - } else if (errno == EAGAIN || errno == EWOULDBLOCK) { - /* - * Return the number of bytes read up to this point if any. - */ - if (index) { - ret = index; - } - goto error; - } else { - PERROR("lttng pipe read"); - goto error; - } - } - read_left -= read_len; - index += read_len; - } while (read_left > 0); - - /* Everything went fine. */ - ret = index; - + ret = lttng_read(pipe->fd[0], buf, count); error: unlock_read_side(pipe); return ret; @@ -310,52 +282,24 @@ error: /* * Write on a lttng pipe using the data in buf and size of count. * - * Return 0 on success or else a negative errno message from write(2). + * Return "count" on success. Return < count on error. errno can be used + * to check the actual error. */ ssize_t lttng_pipe_write(struct lttng_pipe *pipe, const void *buf, size_t count) { - ssize_t ret, write_len, write_left, index; + ssize_t ret; assert(pipe); assert(buf); lock_write_side(pipe); - if (!lttng_pipe_is_write_open(pipe)) { - ret = -EBADF; + ret = -1; + errno = EBADF; goto error; } - - write_left = count; - index = 0; - do { - write_len = write(pipe->fd[1], buf + index, write_left); - if (write_len < 0) { - ret = -errno; - if (errno == EINTR) { - /* Read again. */ - continue; - } else if (errno == EAGAIN || errno == EWOULDBLOCK) { - /* - * Return the number of bytes read up to this point if any. - */ - if (index) { - ret = index; - } - goto error; - } else { - PERROR("lttng pipe write"); - goto error; - } - } - write_left -= write_len; - index += write_len; - } while (write_left > 0); - - /* Everything went fine. */ - ret = index; - + ret = lttng_write(pipe->fd[1], buf, count); error: unlock_write_side(pipe); return ret; diff --git a/src/common/readwrite.c b/src/common/readwrite.c index 43d4e39ee..a559de76b 100644 --- a/src/common/readwrite.c +++ b/src/common/readwrite.c @@ -16,6 +16,8 @@ */ #include +#include +#include #include "readwrite.h" /* @@ -32,7 +34,7 @@ ssize_t lttng_read(int fd, void *buf, size_t count) ssize_t ret; do { - ret = read(fd, &buf[i], count - i); + ret = read(fd, buf + i, count - i); if (ret < 0) { if (errno == EINTR) { continue; /* retry operation */ @@ -59,7 +61,7 @@ ssize_t lttng_write(int fd, const void *buf, size_t count) ssize_t ret; do { - ret = write(fd, &buf[i], count - i); + ret = write(fd, buf + i, count - i); if (ret < 0) { if (errno == EINTR) { continue; /* retry operation */ diff --git a/src/common/runas.c b/src/common/runas.c index 3ae2e4c38..9029f8f74 100644 --- a/src/common/runas.c +++ b/src/common/runas.c @@ -120,11 +120,7 @@ int child_run_as(void *_data) int ret; struct run_as_data *data = _data; ssize_t writelen; - size_t writeleft, index; - union { - int i; - char c[sizeof(int)]; - } sendret; + int sendret; /* * Child: it is safe to drop egid and euid while sharing the @@ -137,7 +133,7 @@ int child_run_as(void *_data) ret = setegid(data->gid); if (ret < 0) { PERROR("setegid"); - sendret.i = -1; + sendret = -1; goto write_return; } } @@ -145,7 +141,7 @@ int child_run_as(void *_data) ret = seteuid(data->uid); if (ret < 0) { PERROR("seteuid"); - sendret.i = -1; + sendret = -1; goto write_return; } } @@ -153,25 +149,17 @@ int child_run_as(void *_data) * Also set umask to 0 for mkdir executable bit. */ umask(0); - sendret.i = (*data->cmd)(data->data); + sendret = (*data->cmd)(data->data); write_return: /* send back return value */ - writeleft = sizeof(sendret); - index = 0; - do { - do { - writelen = write(data->retval_pipe, &sendret.c[index], - writeleft); - } while (writelen < 0 && errno == EINTR); - if (writelen < 0) { - PERROR("write"); - return EXIT_FAILURE; - } - writeleft -= writelen; - index += writelen; - } while (writeleft > 0); - return EXIT_SUCCESS; + writelen = lttng_write(data->retval_pipe, &sendret, sizeof(sendret)); + if (writelen < sizeof(sendret)) { + PERROR("lttng_write error"); + return EXIT_FAILURE; + } else { + return EXIT_SUCCESS; + } } static @@ -179,15 +167,12 @@ int run_as_clone(int (*cmd)(void *data), void *data, uid_t uid, gid_t gid) { struct run_as_data run_as_data; int ret = 0; + ssize_t readlen; int status; pid_t pid; int retval_pipe[2]; - ssize_t readlen, readleft, index; void *child_stack; - union { - int i; - char c[sizeof(int)]; - } retval; + int retval; /* * If we are non-root, we can only deal with our own uid. @@ -203,7 +188,7 @@ int run_as_clone(int (*cmd)(void *data), void *data, uid_t uid, gid_t gid) ret = pipe(retval_pipe); if (ret < 0) { PERROR("pipe"); - retval.i = ret; + retval = ret; goto end; } run_as_data.data = data; @@ -217,7 +202,7 @@ int run_as_clone(int (*cmd)(void *data), void *data, uid_t uid, gid_t gid) -1, 0); if (child_stack == MAP_FAILED) { PERROR("mmap"); - retval.i = -ENOMEM; + retval = -ENOMEM; goto close_pipe; } /* @@ -228,22 +213,14 @@ int run_as_clone(int (*cmd)(void *data), void *data, uid_t uid, gid_t gid) &run_as_data); if (pid < 0) { PERROR("clone"); - retval.i = pid; + retval = pid; goto unmap_stack; } /* receive return value */ - readleft = sizeof(retval); - index = 0; - do { - readlen = read(retval_pipe[0], &retval.c[index], readleft); - if (readlen < 0) { - PERROR("read"); - ret = -1; - break; - } - readleft -= readlen; - index += readlen; - } while (readleft > 0); + readlen = lttng_read(retval_pipe[0], &retval, sizeof(retval)); + if (readlen < sizeof(retval)) { + ret = -1; + } /* * Parent: wait for child to return, in which case the @@ -252,13 +229,13 @@ int run_as_clone(int (*cmd)(void *data), void *data, uid_t uid, gid_t gid) pid = waitpid(pid, &status, 0); if (pid < 0 || !WIFEXITED(status) || WEXITSTATUS(status) != 0) { PERROR("wait"); - retval.i = -1; + retval = -1; } unmap_stack: ret = munmap(child_stack, RUNAS_CHILD_STACK_SIZE); if (ret < 0) { PERROR("munmap"); - retval.i = ret; + retval = ret; } close_pipe: ret = close(retval_pipe[0]); @@ -270,7 +247,7 @@ close_pipe: PERROR("close"); } end: - return retval.i; + return retval; } /* diff --git a/src/common/sessiond-comm/inet.c b/src/common/sessiond-comm/inet.c index fa9218769..2c959c5fe 100644 --- a/src/common/sessiond-comm/inet.c +++ b/src/common/sessiond-comm/inet.c @@ -457,6 +457,7 @@ int lttcomm_close_inet_sock(struct lttcomm_sock *sock) static unsigned long read_proc_value(const char *path) { int ret, fd; + ssize_t size_ret; long r_val; unsigned long val = 0; char buf[64]; @@ -466,11 +467,16 @@ static unsigned long read_proc_value(const char *path) goto error; } - ret = read(fd, buf, sizeof(buf)); - if (ret < 0) { + size_ret = lttng_read(fd, buf, sizeof(buf)); + /* + * Allow reading a file smaller than buf, but keep space for + * final \0. + */ + if (size_ret < 0 || size_ret >= sizeof(buf)) { PERROR("read proc failed"); goto error_close; } + buf[size_ret] = '\0'; errno = 0; r_val = strtol(buf, NULL, 10); diff --git a/src/common/ust-consumer/ust-consumer.c b/src/common/ust-consumer/ust-consumer.c index df0911c71..c6bd5196b 100644 --- a/src/common/ust-consumer/ust-consumer.c +++ b/src/common/ust-consumer/ust-consumer.c @@ -1881,14 +1881,16 @@ int lttng_ustconsumer_read_subbuffer(struct lttng_consumer_stream *stream, /* Ease our life for what's next. */ ustream = stream->ustream; - /* We can consume the 1 byte written into the wait_fd by UST */ + /* + * We can consume the 1 byte written into the wait_fd by UST. + * Don't trigger error if we cannot read this one byte (read + * returns 0), or if the error is EAGAIN or EWOULDBLOCK. + */ if (stream->monitor && !stream->hangup_flush_done) { ssize_t readlen; - do { - readlen = read(stream->wait_fd, &dummy, 1); - } while (readlen == -1 && errno == EINTR); - if (readlen == -1 && errno != EAGAIN && errno != EWOULDBLOCK) { + readlen = lttng_read(stream->wait_fd, &dummy, 1); + if (readlen < 0 && errno != EAGAIN && errno != EWOULDBLOCK) { ret = readlen; goto end; } -- 2.34.1