From e780377f9ffd73c023c458c55e9732b7a7968fe4 Mon Sep 17 00:00:00 2001 From: Michal Bloch Date: Fri, 10 Feb 2023 16:49:34 +0100 Subject: [PATCH] dlog_logger: commonize reader epoll dispatch * behaviour change: reader_logger will now get removed if clock_gettime fails. This doesn't seem like a real concern. * amend commentary to reflect our recent advances in knowledge. Change-Id: Ia4ea04703cd82cf816ef9d6f38f02344cabc4aec Signed-off-by: Michal Bloch --- src/logger/reader_common.c | 25 +++++++++++++++++++++++++ src/logger/reader_common.h | 1 + src/logger/reader_logger.c | 24 +----------------------- src/logger/reader_memory.c | 28 ++-------------------------- src/logger/reader_pipe.c | 32 ++++---------------------------- 5 files changed, 33 insertions(+), 77 deletions(-) diff --git a/src/logger/reader_common.c b/src/logger/reader_common.c index 6faa761..2dc4930 100644 --- a/src/logger/reader_common.c +++ b/src/logger/reader_common.c @@ -78,6 +78,31 @@ void reader_add_sub(struct reader_common *reader, void *sub) list_add(&reader->subs, sub); } +void dispatch_event_reader(struct logger *server, struct epoll_event *event, void *userdata) +{ + struct reader_common *const reader = (struct reader_common *) userdata; + assert(reader); + + if (event->events & (EPOLLHUP | EPOLLERR)) + goto reader_free; + + struct now_t now; + int r = get_now(&now); + if (r < 0) + goto reader_free; + + r = reader->service_reader(reader, now); + if (r != 0) { + /* This can happen for example if a pipe gets clogged, + * and is not necessarily a reason to remove the reader. */ + } + + return; + +reader_free: + reader_free(reader); +} + int reader_flush(struct reader_common *reader, struct timespec now_mono, int flush) { return list_foreach_ret(reader->subs, &(struct subreader_flush_args) { diff --git a/src/logger/reader_common.h b/src/logger/reader_common.h index c3c99f7..d0ebabe 100644 --- a/src/logger/reader_common.h +++ b/src/logger/reader_common.h @@ -64,6 +64,7 @@ void reader_free_ptr(void *reader); void reader_add_sub(struct reader_common *reader, void *sub); void reader_common_init(struct reader_common *reader, struct logger *server); int reader_apply_log_to_subs(struct reader_common *reader, const struct dlogutil_entry *de); +void dispatch_event_reader(struct logger *server, struct epoll_event *event, void *userdata); void subreader_free(void *sub, void *userdata); int subreader_apply_log(void *sub, void *userdata); diff --git a/src/logger/reader_logger.c b/src/logger/reader_logger.c index b57e7ca..a4ea684 100644 --- a/src/logger/reader_logger.c +++ b/src/logger/reader_logger.c @@ -120,28 +120,6 @@ static struct reader_logger *reader_logger_alloc(struct logger *server) return ret; } -static void dispatch_event_reader_logger(struct logger *server, struct epoll_event *event, void *userdata) -{ - struct reader_logger *const rl = (struct reader_logger *) userdata; - assert(rl); - - if (event->events & (EPOLLHUP | EPOLLERR)) { - reader_free(&rl->common); - return; - } - - struct now_t now; - int r = get_now(&now); - if (r < 0) - return; // Don't remove the reader, not its fault. - - r = service_reader_logger(&rl->common, now); - if (r != 0) { - /* TODO: There is no reason not to free the reader in full. However, when I do so, some tests start to - * fail without any reasonable reason. You are welcome to *try* to figure out why does this happen. */ - } -} - int reader_logger_init(struct reader_logger **reader, log_id_t buf_id, struct logger *server, bool skip) { assert(reader); @@ -166,7 +144,7 @@ int reader_logger_init(struct reader_logger **reader, log_id_t buf_id, struct lo ret->skip_count = skip ? logger_get_log_len(ret->device_fd) : 0; - init_fd_entity(&ret->common.fd_entity_source, dispatch_event_reader_logger, ret); + init_fd_entity(&ret->common.fd_entity_source, dispatch_event_reader, ret); set_read_fd_entity(&ret->common.fd_entity_source, ret->device_fd); *reader = ret; diff --git a/src/logger/reader_memory.c b/src/logger/reader_memory.c index ca6c618..35ae5fe 100644 --- a/src/logger/reader_memory.c +++ b/src/logger/reader_memory.c @@ -16,30 +16,6 @@ static void reader_memory_free(struct reader_common *_reader) static int print_out_logs(struct reader_common *_reader, struct now_t _time); -static void dispatch_event_reader_memory(struct logger *server, struct epoll_event *event, void *userdata) -{ - struct reader_memory *const rm = (struct reader_memory *) userdata; - assert(rm); - - if (event->events & (EPOLLHUP | EPOLLERR)) { - reader_free(&rm->common); - return; - } - - struct now_t now; - int r = get_now(&now); - if (r < 0) { - reader_free(&rm->common); - return ; - } - - r = rm->common.service_reader(&rm->common, now); - if (r != 0) { - reader_free(&rm->common); - return; - } -} - static struct reader_memory *reader_memory_alloc(struct logger *server, bool monitor, bool is_dumping) { struct reader_memory *ret = calloc(1, sizeof(*ret)); @@ -106,8 +82,8 @@ int reader_memory_init_with_writer(struct reader_memory **reader, struct writer if (!ret->log_compressed_storage_reader_ptr) return -ENOMEM; - init_fd_entity(&ret->common.fd_entity_sink, dispatch_event_reader_memory, ret); - init_fd_entity(&ret->common.fd_entity_source, dispatch_event_reader_memory, ret); + init_fd_entity(&ret->common.fd_entity_sink, dispatch_event_reader, ret); + init_fd_entity(&ret->common.fd_entity_source, dispatch_event_reader, ret); *reader = ret; ret = NULL; diff --git a/src/logger/reader_pipe.c b/src/logger/reader_pipe.c index 8dd9bbd..1786d6c 100644 --- a/src/logger/reader_pipe.c +++ b/src/logger/reader_pipe.c @@ -14,30 +14,6 @@ static void reader_pipe_free(struct reader_common *_reader) static int print_out_logs(struct reader_common *_reader, struct now_t _time); -static void dispatch_event_reader_pipe(struct logger *server, struct epoll_event *event, void *userdata) -{ - struct reader_pipe *const rp = (struct reader_pipe *) userdata; - assert(rp); - - if (event->events & (EPOLLHUP | EPOLLERR)) { - reader_free(&rp->common); - return; - } - - struct now_t now; - int r = get_now(&now); - if (r < 0) { - reader_free(&rp->common); - return; - } - - r = rp->common.service_reader(&rp->common, now); - if (r != 0) { - /* TODO: There is no reason not to free the reader in full. However, when I do so, some tests start to - * fail without any reasonable reason. You are welcome to *try* to figure out why does this happen. */ - } -} - static struct reader_pipe *reader_pipe_alloc(struct logger *server, bool monitor, bool is_dumping) { struct reader_pipe *ret = calloc(1, sizeof(*ret)); @@ -72,8 +48,8 @@ int reader_pipe_init(struct reader_pipe **reader, log_id_t buf_id, struct logger if (!ret->buf_ptr) return -EINVAL; - init_fd_entity(&ret->common.fd_entity_sink, dispatch_event_reader_pipe, ret); - init_fd_entity(&ret->common.fd_entity_source, dispatch_event_reader_pipe, ret); + init_fd_entity(&ret->common.fd_entity_sink, dispatch_event_reader, ret); + init_fd_entity(&ret->common.fd_entity_source, dispatch_event_reader, ret); *reader = ret; ret = NULL; @@ -94,8 +70,8 @@ int reader_pipe_init_with_writer(struct reader_pipe **reader, struct writer *wri ret->buf_ptr = writer->buf_ptr; - init_fd_entity(&ret->common.fd_entity_sink, dispatch_event_reader_pipe, ret); - init_fd_entity(&ret->common.fd_entity_source, dispatch_event_reader_pipe, ret); + init_fd_entity(&ret->common.fd_entity_sink, dispatch_event_reader, ret); + init_fd_entity(&ret->common.fd_entity_source, dispatch_event_reader, ret); *reader = ret; ret = NULL; -- 2.7.4