From e436e4151ae10349bb89b776fa04a0c21764bf74 Mon Sep 17 00:00:00 2001 From: Michal Bloch Date: Tue, 8 Mar 2022 18:05:56 +0100 Subject: [PATCH] dlog_logger: rearrange readers (no logic change) Preparation for a refactoring of the reader code. Move subreaders away from the Android Logger reader files since they're going to be generic. Change-Id: I00fe88e86db1871886a3a0993b141db56101e84c Signed-off-by: Michal Bloch --- Makefile.am | 2 + src/logger/logger.c | 4 +- src/logger/logger_internal.h | 4 +- src/logger/reader_common.c | 41 ++++++++++- src/logger/reader_common.h | 23 +++++- src/logger/reader_logger.c | 154 ++--------------------------------------- src/logger/reader_logger.h | 20 +----- src/logger/reader_pipe.h | 2 +- src/logger/subreader_file.c | 57 +++++++++++++++ src/logger/subreader_file.h | 10 +++ src/logger/subreader_metrics.c | 53 ++++++++++++++ src/logger/subreader_metrics.h | 10 +++ 12 files changed, 204 insertions(+), 176 deletions(-) create mode 100644 src/logger/subreader_file.c create mode 100644 src/logger/subreader_file.h create mode 100644 src/logger/subreader_metrics.c create mode 100644 src/logger/subreader_metrics.h diff --git a/Makefile.am b/Makefile.am index 32fe5c6..34a98af 100644 --- a/Makefile.am +++ b/Makefile.am @@ -135,6 +135,8 @@ dlog_logger_SOURCES = \ src/logger/reader_common.c \ src/logger/reader_logger.c \ src/logger/reader_pipe.c \ + src/logger/subreader_file.c \ + src/logger/subreader_metrics.c \ src/logger/socket.c \ src/logger/writer.c \ src/shared/backend_androidlogger.c \ diff --git a/src/logger/logger.c b/src/logger/logger.c index 717d373..c964b77 100644 --- a/src/logger/logger.c +++ b/src/logger/logger.c @@ -20,6 +20,8 @@ #include "logger_internal.h" #include "logger_privileges.h" +#include "subreader_file.h" +#include "subreader_metrics.h" #include #include @@ -439,7 +441,7 @@ int service_writer_kmsg(struct logger *server, struct writer *wr, struct epoll_e return 0; } -void remove_reader_fd_entities(struct logger *server, struct reader *reader) +void remove_reader_fd_entities(struct logger *server, struct reader_common *reader) { assert(reader); diff --git a/src/logger/logger_internal.h b/src/logger/logger_internal.h index 28aa8d2..b65b149 100644 --- a/src/logger/logger_internal.h +++ b/src/logger/logger_internal.h @@ -155,8 +155,8 @@ struct logger_config_data { char *first_time_file_path; }; -void remove_reader_fd_entities(struct logger *server, struct reader *reader); -void reader_deinit_common(struct reader *reader); +void remove_reader_fd_entities(struct logger *server, struct reader_common *reader); +void reader_deinit_common(struct reader_common *reader); void check_if_fd_limit_reached(struct logger *server, int err); int service_writer_kmsg(struct logger *server, struct writer *wr, struct epoll_event *event); int service_writer_socket(struct logger *server, struct writer *wr, struct epoll_event *event); diff --git a/src/logger/reader_common.c b/src/logger/reader_common.c index 711895b..f97ca75 100644 --- a/src/logger/reader_common.c +++ b/src/logger/reader_common.c @@ -1,8 +1,10 @@ #include "reader_common.h" +#include "subreader_file.h" +#include "subreader_metrics.h" #include -void reader_deinit_common(struct reader *reader) +void reader_deinit_common(struct reader_common *reader) { if (reader->fd_entity_sink.fd >= 0) close(reader->fd_entity_sink.fd); @@ -10,3 +12,40 @@ void reader_deinit_common(struct reader *reader) close(reader->fd_entity_source.fd); } +void subreader_free(void *_sub, void *userdata) +{ + struct subreader_common *const sub = (struct subreader_common *) _sub; + assert(sub); + assert(userdata == NULL); + + sub->sub_destroy(sub->sub_userdata); + if (sub->filter) + log_filter_free(sub->filter); + free(sub->sub_userdata); +} + +void subreader_apply_log(void *_sub, void *userdata) +{ + struct subreader_common *const sub = (struct subreader_common *) _sub; + assert(sub); + + const struct dlogutil_entry *const due = (const struct dlogutil_entry *) userdata; + assert(due); + + if (!log_should_print_line(sub->filter, due)) + return; + + sub->sub_apply_log(sub, due); +} + +void subreader_flush(void *_sub, void *userdata) +{ + struct subreader_common *const sub = (struct subreader_common *) _sub; + assert(sub); + assert(sub->sub_flush); + + struct subreader_flush_args *const args = (struct subreader_flush_args *) userdata; + + sub->sub_flush(sub->sub_userdata, args->ts, args->flush_time); +} + diff --git a/src/logger/reader_common.h b/src/logger/reader_common.h index d0bc6a4..07bcb32 100644 --- a/src/logger/reader_common.h +++ b/src/logger/reader_common.h @@ -2,9 +2,28 @@ #include "fd_entity.h" -struct reader { +struct dlogutil_entry; + +struct reader_common { struct fd_entity fd_entity_sink; struct fd_entity fd_entity_source; }; -void reader_deinit_common(struct reader *reader); +struct subreader_common { + void (*sub_apply_log)(const struct subreader_common *sub, const struct dlogutil_entry *due); + void (*sub_flush)(void *sub_userdata, struct timespec ts, int flush_time); + void (*sub_destroy)(void *sub_userdata); + void *sub_userdata; + struct log_filter *filter; +}; + +struct subreader_flush_args { + struct timespec ts; + int flush_time; +}; + +void reader_deinit_common(struct reader_common *reader); + +void subreader_free(void *sub, void *userdata); +void subreader_apply_log(void *sub, void *userdata); +void subreader_flush(void *sub, void *userdata); diff --git a/src/logger/reader_logger.c b/src/logger/reader_logger.c index e4eab9b..a050188 100644 --- a/src/logger/reader_logger.c +++ b/src/logger/reader_logger.c @@ -1,105 +1,13 @@ #include "reader_logger.h" #include "logger_internal.h" -static void subreader_logger_metrics_apply_log(const struct subreader_logger *srl, const struct dlogutil_entry *due) -{ - assert(srl); - assert(due); - - struct subreader_logger_metrics *const srlm = (struct subreader_logger_metrics *) srl->sub_userdata; - assert(srlm); - - qos_add_log(srlm->qos, due); -} - -static void subreader_logger_metrics_free(void *userdata) -{ - // nothing to do; we're just a wrapper over a weak (shared) pointer -} - -static void subreader_logger_metrics_flush(void *userdata, struct timespec ts, int flush_time) -{ - // nothing to do either; no such concept as flushing metrics -} - -static void subreader_logger_file_apply_log(const struct subreader_logger *srl, const struct dlogutil_entry *due) -{ - assert(srl); - assert(due); - - struct subreader_logger_file *const srlf = (struct subreader_logger_file *) srl->sub_userdata; - assert(srlf); - - if (logfile_write_with_rotation(due, &srlf->file, DLOGUTIL_SORT_SENT_REAL)) { - // ignore errors, can't do anything about them - } -} - -static void subreader_logger_file_free(void *userdata) -{ - struct subreader_logger_file *const srlf = (struct subreader_logger_file *) userdata; - assert(srlf); - - logfile_free(&srlf->file); -} - -static void subreader_logger_file_flush(void *userdata, struct timespec ts, int flush_time) -{ - struct subreader_logger_file *const srlf = (struct subreader_logger_file *) userdata; - assert(srlf); - - flush_logfile_timely(&srlf->file, ts, flush_time); -} - -static void subreader_logger_free(void *sub, void *userdata) -{ - struct subreader_logger *const srl = (struct subreader_logger *) sub; - assert(srl); - assert(userdata == NULL); - - srl->sub_destroy(srl->sub_userdata); - if (srl->filter) - log_filter_free(srl->filter); - free(srl->sub_userdata); -} - -static void subreader_logger_apply_log(void *sub, void *userdata) -{ - struct subreader_logger *const srl = (struct subreader_logger *) sub; - assert(srl); - - const struct dlogutil_entry *const due = (const struct dlogutil_entry *) userdata; - assert(due); - - if (!log_should_print_line(srl->filter, due)) - return; - - srl->sub_apply_log(srl, due); -} - -struct flush_args { - struct timespec ts; - int flush_time; -}; - -static void subreader_logger_flush(void *sub, void *userdata) -{ - struct subreader_logger *const srl = (struct subreader_logger *) sub; - assert(srl); - assert(srl->sub_flush); - - struct flush_args *const args = (struct flush_args *) userdata; - - srl->sub_flush(srl->sub_userdata, args->ts, args->flush_time); -} - void reader_logger_free(struct reader_logger *reader) { if (!reader) return; reader_deinit_common(&reader->common); - list_clear_custom(&reader->subs, NULL, subreader_logger_free); + list_clear_custom(&reader->subs, NULL, subreader_free); free(reader); } @@ -175,7 +83,7 @@ int service_reader_logger(struct reader_logger *reader, struct now_t time) // TODO: Consider calling this in a more robust way (and not having pipe in the name) fixup_pipe_msg(&entry, r - sizeof(*ale)); - list_foreach(reader->subs, &entry.header, subreader_logger_apply_log); + list_foreach(reader->subs, &entry.header, subreader_apply_log); } } @@ -193,60 +101,6 @@ static struct reader_logger *reader_logger_alloc(void) return ret; } -int reader_logger_add_subreader_file(struct reader_logger *reader, struct log_filter *filter, struct log_file *file) -{ - assert(reader); - assert(filter); - - struct subreader_logger *const srl = malloc(sizeof *srl); - struct subreader_logger_file *const srlf = malloc(sizeof *srlf); - if (!srl || !srlf) { - free(srl); - free(srlf); - return -ENOMEM; - } - - logfile_move(&srlf->file, file); - - srl->sub_userdata = srlf; - srl->sub_destroy = subreader_logger_file_free; - srl->sub_apply_log = subreader_logger_file_apply_log; - srl->sub_flush = subreader_logger_file_flush; - srl->filter = log_filter_move(filter); - - list_add(&reader->subs, srl); - - return 0; -} - -int reader_logger_add_subreader_metrics(struct reader_logger *reader, struct qos_module *qos) -{ - assert(reader); - assert(qos); - - struct subreader_logger *const srl = malloc(sizeof *srl); - struct subreader_logger_metrics *const srlm = malloc(sizeof *srlm); - struct log_filter *const filter = log_filter_new(); - if (!srl || !srlm || !filter || log_filter_set_filterspec(filter, "*:V")) { - free(srl); - free(srlm); - log_filter_free(filter); - return -ENOMEM; - } - - srlm->qos = qos; - - srl->sub_userdata = srlm; - srl->sub_destroy = subreader_logger_metrics_free; - srl->sub_apply_log = subreader_logger_metrics_apply_log; - srl->sub_flush = subreader_logger_metrics_flush; - srl->filter = filter; - - list_add(&reader->subs, srl); - - return 0; -} - static void dispatch_event_reader_logger(struct logger *server, struct epoll_event *event, void *userdata) { struct reader_logger *const rl = (struct reader_logger *) userdata; @@ -308,8 +162,8 @@ int reader_logger_init(struct reader_logger **reader, log_id_t buf_id, struct lo void reader_logger_flush(struct reader_logger *reader, struct timespec now_mono, int flush) { - list_foreach(reader->subs, &(struct flush_args) { + list_foreach(reader->subs, &(struct subreader_flush_args) { .ts = now_mono, .flush_time = flush, - }, subreader_logger_flush); + }, subreader_flush); } diff --git a/src/logger/reader_logger.h b/src/logger/reader_logger.h index 14ef70f..53c01b8 100644 --- a/src/logger/reader_logger.h +++ b/src/logger/reader_logger.h @@ -4,33 +4,15 @@ #include #include "reader_common.h" -struct subreader_logger_file { - struct log_file file; -}; - -struct subreader_logger_metrics { - struct qos_module *qos; -}; - -struct subreader_logger { - void (*sub_apply_log)(const struct subreader_logger *srl, const struct dlogutil_entry *due); - void (*sub_flush)(void *sub_userdata, struct timespec ts, int flush_time); - void (*sub_destroy)(void *sub_userdata); - void *sub_userdata; - struct log_filter *filter; -}; - struct reader_logger { list_head subs; log_id_t buf_id; int skip_count; - struct reader common; + struct reader_common common; }; int reader_logger_init(struct reader_logger **reader, log_id_t buf_id, struct logger *server, bool skip); int service_reader_logger(struct reader_logger *reader, struct now_t time); -int reader_logger_add_subreader_file(struct reader_logger *reader, struct log_filter *filter, struct log_file *file); -int reader_logger_add_subreader_metrics(struct reader_logger *reader, struct qos_module *qos); void reader_logger_free(struct reader_logger *reader); void reader_logger_cleanup(struct reader_logger *const *ptr); void reader_logger_flush(struct reader_logger *reader, struct timespec now_mono, int flush); diff --git a/src/logger/reader_pipe.h b/src/logger/reader_pipe.h index 2c0d31f..d517795 100644 --- a/src/logger/reader_pipe.h +++ b/src/logger/reader_pipe.h @@ -19,7 +19,7 @@ struct reader_pipe { bool is_dumping; bool monitor; struct log_filter *filter; - struct reader common; + struct reader_common common; }; int reader_pipe_init(struct reader_pipe **reader, log_id_t buf_id, struct logger *server, diff --git a/src/logger/subreader_file.c b/src/logger/subreader_file.c new file mode 100644 index 0000000..45e51fa --- /dev/null +++ b/src/logger/subreader_file.c @@ -0,0 +1,57 @@ +#include "subreader_file.h" +#include "logger_internal.h" + +static void subreader_file_apply_log(const struct subreader_common *sub, const struct dlogutil_entry *due) +{ + assert(sub); + assert(due); + + struct subreader_file *const srf = (struct subreader_file *) sub->sub_userdata; + assert(srf); + + if (logfile_write_with_rotation(due, &srf->file, DLOGUTIL_SORT_SENT_REAL)) { + // ignore errors, can't do anything about them + } +} + +static void subreader_file_free(void *userdata) +{ + struct subreader_file *const srf = (struct subreader_file *) userdata; + assert(srf); + + logfile_free(&srf->file); +} + +static void subreader_file_flush(void *userdata, struct timespec ts, int flush_time) +{ + struct subreader_file *const srf = (struct subreader_file *) userdata; + assert(srf); + + flush_logfile_timely(&srf->file, ts, flush_time); +} + +int reader_logger_add_subreader_file(struct reader_logger *reader, struct log_filter *filter, struct log_file *file) +{ + assert(reader); + assert(filter); + + struct subreader_common *const sub = malloc(sizeof *sub); + struct subreader_file *const srf = malloc(sizeof *srf); + if (!sub || !srf) { + free(sub); + free(srf); + return -ENOMEM; + } + + logfile_move(&srf->file, file); + + sub->sub_userdata = srf; + sub->sub_destroy = subreader_file_free; + sub->sub_apply_log = subreader_file_apply_log; + sub->sub_flush = subreader_file_flush; + sub->filter = log_filter_move(filter); + + list_add(&reader->subs, sub); + + return 0; +} diff --git a/src/logger/subreader_file.h b/src/logger/subreader_file.h new file mode 100644 index 0000000..e7f06b5 --- /dev/null +++ b/src/logger/subreader_file.h @@ -0,0 +1,10 @@ +#pragma once + +#include "reader_logger.h" + +struct subreader_file { + struct log_file file; +}; + +int reader_logger_add_subreader_file(struct reader_logger *reader, struct log_filter *filter, struct log_file *file); + diff --git a/src/logger/subreader_metrics.c b/src/logger/subreader_metrics.c new file mode 100644 index 0000000..6f7880d --- /dev/null +++ b/src/logger/subreader_metrics.c @@ -0,0 +1,53 @@ +#include "subreader_metrics.h" +#include "qos.h" + +static void subreader_metrics_apply_log(const struct subreader_common *sub, const struct dlogutil_entry *due) +{ + assert(sub); + assert(due); + + struct subreader_metrics *const srm = (struct subreader_metrics *) sub->sub_userdata; + assert(srm); + + qos_add_log(srm->qos, due); +} + +static void subreader_metrics_free(void *userdata) +{ + // nothing to do; we're just a wrapper over a weak (shared) pointer +} + +static void subreader_metrics_flush(void *userdata, struct timespec ts, int flush_time) +{ + // nothing to do either; no such concept as flushing metrics +} + + +int reader_logger_add_subreader_metrics(struct reader_logger *reader, struct qos_module *qos) +{ + assert(reader); + assert(qos); + + struct subreader_common *const sub = malloc(sizeof *sub); + struct subreader_metrics *const srm = malloc(sizeof *srm); + struct log_filter *const filter = log_filter_new(); + if (!sub || !srm || !filter || log_filter_set_filterspec(filter, "*:V")) { + free(sub); + free(srm); + log_filter_free(filter); + return -ENOMEM; + } + + srm->qos = qos; + + sub->sub_userdata = srm; + sub->sub_destroy = subreader_metrics_free; + sub->sub_apply_log = subreader_metrics_apply_log; + sub->sub_flush = subreader_metrics_flush; + sub->filter = filter; + + list_add(&reader->subs, sub); + + return 0; +} + diff --git a/src/logger/subreader_metrics.h b/src/logger/subreader_metrics.h new file mode 100644 index 0000000..98972f1 --- /dev/null +++ b/src/logger/subreader_metrics.h @@ -0,0 +1,10 @@ +#pragma once + +#include "reader_logger.h" + +struct subreader_metrics { + struct qos_module *qos; +}; + +int reader_logger_add_subreader_metrics(struct reader_logger *reader, struct qos_module *qos); + -- 2.7.4