From 13791078328c76ea7b99667108b9c798c31f8d68 Mon Sep 17 00:00:00 2001 From: Mateusz Majewski Date: Tue, 18 Feb 2020 08:22:10 +0100 Subject: [PATCH] Refactor FDI EOF signalisation Change-Id: I235a4176a7b0966f2be46c0088cd062ac0239e81 --- include/fd_info.h | 3 +++ include/fdi_pipe_internal.h | 1 + src/libdlogutil/fdi_logger.c | 21 +++++++++++++++------ src/libdlogutil/fdi_pipe.c | 12 ++++++++++++ src/libdlogutil/logretrieve.c | 22 +++++++++++++++++----- src/tests/fdi_logger.c | 6 +++--- 6 files changed, 51 insertions(+), 14 deletions(-) diff --git a/include/fd_info.h b/include/fd_info.h index 6d32674..bfd7fd4 100644 --- a/include/fd_info.h +++ b/include/fd_info.h @@ -62,6 +62,9 @@ struct fd_ops { /// Get the size of the buffer int (*getsize)(struct fd_info *fdi, unsigned int *val); + /// Check whether there will be no more logs on this FD + bool (*eof)(struct fd_info *fdi); + // Whether to filter logs on the client bool should_filter; }; diff --git a/include/fdi_pipe_internal.h b/include/fdi_pipe_internal.h index 2492e41..0b5e35e 100644 --- a/include/fdi_pipe_internal.h +++ b/include/fdi_pipe_internal.h @@ -7,5 +7,6 @@ struct pipe_priv_data { size_t offset; char buff[RECEIVE_BUFFER_SIZE]; int sock_fd; + bool eof; }; diff --git a/src/libdlogutil/fdi_logger.c b/src/libdlogutil/fdi_logger.c index ebc9e2b..77045db 100644 --- a/src/libdlogutil/fdi_logger.c +++ b/src/libdlogutil/fdi_logger.c @@ -229,6 +229,15 @@ static int logger_prepare_print(struct fd_info *fdi, int dump, bool monitor, dlo return 0; } +static bool logger_eof(struct fd_info *fdi) +{ + assert(fdi); + struct logger_priv_data *lpd = (struct logger_priv_data *)fdi->priv_data; + assert(lpd); + + return lpd->log_len <= 0 && lpd->log_len != UNLIMITED_LOG_LEN && !lpd->monitor; +} + static int logger_read(struct fd_info *fdi) { assert(fdi); @@ -238,6 +247,9 @@ static int logger_read(struct fd_info *fdi) if (lpd->entry) return -EAGAIN; + if (logger_eof(fdi)) + return 0; + char buff[LOG_MAX_PAYLOAD_SIZE + sizeof(struct android_logger_entry)]; if (lpd->monitor) { assert(lpd->log_len != UNLIMITED_LOG_LEN); @@ -270,12 +282,8 @@ static int logger_read(struct fd_info *fdi) lpd->entry->sec_recv_real = INVALID_SEC; lpd->entry->nsec_recv_real = INVALID_NSEC; - if (lpd->log_len != UNLIMITED_LOG_LEN) { - lpd->log_len -= r; - - if (lpd->log_len <= 0) - return 0; // emulate EOF - } + if (lpd->log_len != UNLIMITED_LOG_LEN) + lpd->log_len -= r < lpd->log_len ? r : lpd->log_len; return r; } @@ -319,5 +327,6 @@ struct fd_ops ops_logger = { .prepare_print = logger_prepare_print, .clear = logger_clear, .getsize = logger_getsize, + .eof = logger_eof, .should_filter = true, }; diff --git a/src/libdlogutil/fdi_pipe.c b/src/libdlogutil/fdi_pipe.c index 078ede2..06e0e6f 100644 --- a/src/libdlogutil/fdi_pipe.c +++ b/src/libdlogutil/fdi_pipe.c @@ -179,6 +179,7 @@ static int pipe_create(struct fd_info *fdi, const struct log_config *conf, const ppd->data_len = 0; ppd->sock_fd = sock_fd; ppd->offset = 0; + ppd->eof = false; fdi->priv_data = ppd; return 0; @@ -237,6 +238,7 @@ static int pipe_read(struct fd_info *fdi) return -errno; ppd->data_len += r; + ppd->eof |= r == 0; return r; } @@ -280,6 +282,15 @@ static const dlogutil_entry_s *pipe_peek_entry(const struct fd_info *fdi) return (const dlogutil_entry_s *)(ppd->buff + ppd->offset); } +static bool pipe_eof(struct fd_info *fdi) +{ + assert(fdi); + const struct pipe_priv_data *const ppd = (const struct pipe_priv_data *)fdi->priv_data; + assert(ppd); + + return ppd->eof; +} + struct fd_ops ops_pipe = { .create = pipe_create, .destroy = pipe_destroy, @@ -290,5 +301,6 @@ struct fd_ops ops_pipe = { .prepare_print = pipe_prepare_print, .clear = pipe_clear, .getsize = pipe_getsize, + .eof = pipe_eof, .should_filter = false, }; diff --git a/src/libdlogutil/logretrieve.c b/src/libdlogutil/logretrieve.c index 51ca2ac..a555623 100644 --- a/src/libdlogutil/logretrieve.c +++ b/src/libdlogutil/logretrieve.c @@ -165,6 +165,10 @@ int do_print(struct fd_info **data_fds, int fd_count, dlogutil_entry_cb callback if (!sort_vector_finalize(&logs)) return -ENOMEM; + __attribute__((cleanup(free_ptr))) bool *enabled = calloc(fd_count, sizeof *enabled); + if (!enabled) + return -ENOMEM; + for (int nfds = 0; nfds < fd_count; ++nfds) { int r = data_fds[nfds]->ops->prepare_print(data_fds[nfds], logs.dump, opt->monitor, opt->filter_object); if (r < 0) @@ -183,6 +187,7 @@ int do_print(struct fd_info **data_fds, int fd_count, dlogutil_entry_cb callback .data.ptr = data_fds[nfds], .events = EPOLLIN, }; + enabled[nfds] = true; epoll_ctl(epollfd, EPOLL_CTL_ADD, fd, &ev); ++epoll_cnt; } @@ -205,11 +210,6 @@ int do_print(struct fd_info **data_fds, int fd_count, dlogutil_entry_cb callback else break; } - - if (r == 0) { - epoll_ctl(epollfd, EPOLL_CTL_DEL, fdi->fd, NULL); - --epoll_cnt; - } } int r = put_logs_into_vector(data_fds, fd_count, nfds, &logs, callback, userdata, opt->filter_object); @@ -230,6 +230,18 @@ int do_print(struct fd_info **data_fds, int fd_count, dlogutil_entry_cb callback if (r != 0) return r; } + + /* Instead of removing buffers when ->read returns 0, we do it + * in a separate loop, using their ->eof "method". This is because + * removing buffers that way would fail if EOF would be triggered + * somewhere else and the buffer would never be ->read again. */ + for (int i = 0; i < fd_count; ++i) { + if (enabled[i] && data_fds[i]->ops->eof(data_fds[i])) { + enabled[i] = false; + epoll_ctl(epollfd, EPOLL_CTL_DEL, data_fds[i]->fd, NULL); + --epoll_cnt; + } + } } int r = put_logs_into_vector(data_fds, fd_count, 0, &logs, callback, userdata, opt->filter_object); diff --git a/src/tests/fdi_logger.c b/src/tests/fdi_logger.c index 0f5b3f4..38fba25 100644 --- a/src/tests/fdi_logger.c +++ b/src/tests/fdi_logger.c @@ -179,9 +179,6 @@ int main() assert(0 == ops_logger.getsize(&fdi, &sz)); assert(2019 == sz); - expected_ioctl = -1; - assert(!ops_logger.prepare_print(&fdi, DLOGUTIL_MODE_CONTINUOUS, false, (dlogutil_filter_options_s *) 0xCA5CADE)); - expected_ioctl = LOGGER_GET_LOG_LEN; ioctl_ret = -67; assert(-67 == ops_logger.prepare_print(&fdi, 123, false, (dlogutil_filter_options_s *) 0xCA5CADE)); @@ -189,6 +186,9 @@ int main() ioctl_ret = 0; assert(1 == ops_logger.prepare_print(&fdi, 123, false, (dlogutil_filter_options_s *) 0xCA5CADE)); + expected_ioctl = -1; + assert(!ops_logger.prepare_print(&fdi, DLOGUTIL_MODE_CONTINUOUS, false, (dlogutil_filter_options_s *) 0xCA5CADE)); + fail_read = true; assert(-334 == ops_logger.read(&fdi)); fail_read = false; -- 2.7.4