From c11ca003b6b7e65cc4ebfabc7b898ba452e02114 Mon Sep 17 00:00:00 2001 From: Michal Bloch Date: Tue, 28 Jul 2020 16:46:46 +0200 Subject: [PATCH] dlog_logger: move fields to reader child classes Makes the generic reader struct less of a kitchen sink. Change-Id: I57fca12ae9439c22e0c95afb41d5ef8478a6cbf0 Signed-off-by: Michal Bloch --- src/logger/logger.c | 76 ++++++++++++++++++++------------------------ src/logger/logger_internal.h | 7 ++-- src/tests/logger.c | 12 +++---- 3 files changed, 44 insertions(+), 51 deletions(-) diff --git a/src/logger/logger.c b/src/logger/logger.c index effa11a..051bb3d 100644 --- a/src/logger/logger.c +++ b/src/logger/logger.c @@ -53,7 +53,7 @@ static volatile sig_atomic_t g_logger_run = 1; static struct { int (*reader_any_init)(struct reader_any *reader, log_id_t log_id, struct logger *server, - dlogutil_filter_options_s *filter, struct log_file file, bool monitor, int dumpcount); + dlogutil_filter_options_s *filter, struct log_file file, bool monitor, bool is_dumping); char logger_devices[LOG_ID_MAX][MAX_CONF_VAL_LEN]; int logger_device_throttling[LOG_ID_MAX]; } g_backend = { @@ -593,7 +593,7 @@ uint64_t reader_buffered_space(const struct reader_pipe *reader) int reader_is_bufferable(const struct reader_pipe *reader) { assert(reader); - return reader->buf_ptr && reader->common.file.path != NULL; + return reader->buf_ptr && reader->file.path != NULL; } int reader_ms_since(const struct reader_pipe *reader, struct timespec *ts) @@ -677,8 +677,8 @@ static int reader_print_out_single_log(struct reader_pipe *reader, const dloguti if (!log_should_print_line(reader->common.filter, dlogutil_entry)) return 0; - if (reader->common.file.path) { - logfile_write_with_rotation(dlogutil_entry, &reader->common.file, reader->buf_ptr->sort_by); + if (reader->file.path) { + logfile_write_with_rotation(dlogutil_entry, &reader->file, reader->buf_ptr->sort_by); return 0; } @@ -686,7 +686,7 @@ static int reader_print_out_single_log(struct reader_pipe *reader, const dloguti if (!strlen(tag)) return 0; - int r = write(reader->common.file.path ? reader->common.file.fd : reader->common.fd_entity_sink.fd, dlogutil_entry, dlogutil_entry->len); + int r = write(reader->file.path ? reader->file.fd : reader->common.fd_entity_sink.fd, dlogutil_entry, dlogutil_entry->len); if (r < 0) { if (errno != EAGAIN) return 1; @@ -696,13 +696,13 @@ static int reader_print_out_single_log(struct reader_pipe *reader, const dloguti r = 0; } - reader->common.file.size += r; + reader->file.size += r; if (r < dlogutil_entry->len) { reader->partial_log_size = dlogutil_entry->len - r; memcpy(reader->partial_log, ((char *)dlogutil_entry) + r, reader->partial_log_size); return -1; - } else if (logfile_rotate_needed(&reader->common.file) > 0) { - logfile_do_rotate(&reader->common.file); + } else if (logfile_rotate_needed(&reader->file) > 0) { + logfile_do_rotate(&reader->file); } return 0; @@ -754,7 +754,7 @@ int print_out_logs(struct reader_pipe *reader, struct now_t _time) } } - if (reader->common.dumpcount) + if (reader->is_dumping) ret = 1; else ret = -1; @@ -798,7 +798,6 @@ static void reader_deinit_common(struct reader *reader) close(reader->fd_entity_sink.fd); if (reader->fd_entity_source.fd >= 0) close(reader->fd_entity_source.fd); - logfile_free(&reader->file); if (reader->filter) log_filter_free(reader->filter); } @@ -809,6 +808,7 @@ void reader_pipe_free(struct reader_pipe *reader) return; reader_deinit_common(&reader->common); + logfile_free(&reader->file); if (reader->log_storage_reader) log_storage_release_reader(reader->log_storage_reader); @@ -821,6 +821,7 @@ static void reader_logger_free(struct reader_logger *reader) return; reader_deinit_common(&reader->common); + logfile_free(&reader->file); free(reader); } @@ -889,7 +890,7 @@ int service_reader_logger(struct reader_logger* reader, struct now_t time) if (!log_should_print_line(reader->common.filter, &entry.header)) continue; - logfile_write_with_rotation(&entry.header, &reader->common.file, DLOGUTIL_SORT_SENT_REAL); + logfile_write_with_rotation(&entry.header, &reader->file, DLOGUTIL_SORT_SENT_REAL); } } @@ -917,7 +918,7 @@ int add_buffer_reader(struct log_buffer *buffer, struct reader_pipe *reader) assert(buffer); reader->log_storage_reader = log_storage_new_reader(buffer->log_storage, - reader->common.dumpcount, reader->common.monitor, reader_notify_losing_log, reader); + reader->is_dumping, reader->monitor, reader_notify_losing_log, reader); if (!reader->log_storage_reader) return -ENOMEM; @@ -1014,18 +1015,16 @@ static int add_reader_any(struct logger *server, struct reader_any reader) } static struct reader_pipe *reader_pipe_init(dlogutil_filter_options_s *filter, struct log_file file, struct timespec ts, - bool monitor, int dumpcount) + bool monitor, bool is_dumping) { struct reader_pipe *ret = calloc(1, sizeof(*ret)); if (!ret) return NULL; - ret->common = (struct reader) { - .filter = filter, - .file = file, - .monitor = monitor, - .dumpcount = dumpcount, - }; + ret->common.filter = filter; + ret->monitor = monitor; + ret->is_dumping = is_dumping; + ret->file = file; ret->buf_ptr = NULL; ret->log_storage_reader = NULL; ret->last_read_time = ts; @@ -1034,26 +1033,21 @@ static struct reader_pipe *reader_pipe_init(dlogutil_filter_options_s *filter, s return ret; } -static struct reader_logger *reader_logger_init(dlogutil_filter_options_s *filter, struct log_file file, struct timespec ts, - bool monitor, int dumpcount) +static struct reader_logger *reader_logger_init(dlogutil_filter_options_s *filter, struct log_file file) { struct reader_logger *ret = calloc(1, sizeof(*ret)); if (!ret) return NULL; - ret->common = (struct reader) { - .filter = filter, - .file = file, - .monitor = monitor, - .dumpcount = dumpcount, - }; + ret->common.filter = filter; + ret->file = file; ret->buf_id = LOG_ID_INVALID; return ret; } static int reader_any_init_for_pipe(struct reader_any *reader, log_id_t buf_id, struct logger *server, - dlogutil_filter_options_s *filter, struct log_file file, bool monitor, int dumpcount) + dlogutil_filter_options_s *filter, struct log_file file, bool monitor, bool is_dumping) { assert(reader); assert(buf_id > LOG_ID_INVALID); @@ -1061,7 +1055,7 @@ static int reader_any_init_for_pipe(struct reader_any *reader, log_id_t buf_id, assert(server); assert(filter); - __attribute__((cleanup(reader_pipe_cleanup))) struct reader_pipe *ret = reader_pipe_init(filter, file, server->time.mono, monitor, dumpcount); + __attribute__((cleanup(reader_pipe_cleanup))) struct reader_pipe *ret = reader_pipe_init(filter, file, server->time.mono, monitor, is_dumping); if (!ret) return -ENOMEM; @@ -1079,7 +1073,7 @@ static int reader_any_init_for_pipe(struct reader_any *reader, log_id_t buf_id, } static int reader_pipe_init_with_writer(struct reader_pipe **reader, struct writer *writer, struct logger *server, - dlogutil_filter_options_s *filter, struct log_file file, bool monitor, int dumpcount) + dlogutil_filter_options_s *filter, struct log_file file, bool monitor, bool is_dumping) { assert(reader); assert(writer); @@ -1087,7 +1081,7 @@ static int reader_pipe_init_with_writer(struct reader_pipe **reader, struct writ assert(server); assert(filter); - __attribute__((cleanup(reader_pipe_cleanup))) struct reader_pipe *ret = reader_pipe_init(filter, file, server->time.mono, monitor, dumpcount); + __attribute__((cleanup(reader_pipe_cleanup))) struct reader_pipe *ret = reader_pipe_init(filter, file, server->time.mono, monitor, is_dumping); if (!ret) return -ENOMEM; @@ -1102,7 +1096,7 @@ static int reader_pipe_init_with_writer(struct reader_pipe **reader, struct writ } static int reader_any_init_for_logger(struct reader_any *reader, log_id_t buf_id, struct logger *server, - dlogutil_filter_options_s *filter, struct log_file file, bool monitor, int dumpcount) + dlogutil_filter_options_s *filter, struct log_file file, bool monitor, bool is_dumping) { assert(reader); assert(buf_id > LOG_ID_INVALID); @@ -1111,9 +1105,9 @@ static int reader_any_init_for_logger(struct reader_any *reader, log_id_t buf_id assert(filter); if (buf_id == LOG_ID_KMSG || buf_id == LOG_ID_SYSLOG) - return reader_any_init_for_pipe(reader, buf_id, server, filter, file, monitor, dumpcount); + return reader_any_init_for_pipe(reader, buf_id, server, filter, file, monitor, is_dumping); - __attribute__((cleanup(reader_logger_cleanup))) struct reader_logger *ret = reader_logger_init(filter, file, server->time.mono, monitor, dumpcount); + __attribute__((cleanup(reader_logger_cleanup))) struct reader_logger *ret = reader_logger_init(filter, file); if (!ret) return -ENOMEM; @@ -1227,7 +1221,7 @@ static int create_reader_from_dlogutil_line(const char *cmdl, struct writer *wr, struct log_file file; logfile_init(&file); - bool dumpcount = false; + bool is_dumping = false; bool monitor = false; log_id_t buf_id = LOG_ID_INVALID; @@ -1259,7 +1253,7 @@ static int create_reader_from_dlogutil_line(const char *cmdl, struct writer *wr, break; case 'd': case 't': - dumpcount = true; + is_dumping = true; break; case 'm': monitor = true; @@ -1337,8 +1331,8 @@ static int create_reader_from_dlogutil_line(const char *cmdl, struct writer *wr, } // dump + monitor = continuous - if (monitor && dumpcount) { - dumpcount = false; + if (monitor && is_dumping) { + is_dumping = false; monitor = false; } @@ -1356,12 +1350,12 @@ static int create_reader_from_dlogutil_line(const char *cmdl, struct writer *wr, if (wr && wr->buf_ptr) { struct reader_pipe *pipe = NULL; - retval = reader_pipe_init_with_writer(&pipe, wr, server, filter, file, monitor, dumpcount); + retval = reader_pipe_init_with_writer(&pipe, wr, server, filter, file, monitor, is_dumping); reader.which = READER_ANY_PIPE; reader.pipe_inner = pipe; } else if (buf_id != LOG_ID_INVALID) - retval = g_backend.reader_any_init(&reader, buf_id, server, filter, file, monitor, dumpcount); + retval = g_backend.reader_any_init(&reader, buf_id, server, filter, file, monitor, is_dumping); else retval = -EINVAL; if (retval != 0) @@ -1377,7 +1371,7 @@ static int create_reader_from_dlogutil_line(const char *cmdl, struct writer *wr, if (wr) { assert(reader.which == READER_ANY_PIPE); int write_fd = -1, read_fd = -1; - retval = create_fifo_fds(server, wr->fd_entity.fd, &write_fd, &read_fd, reader.pipe_inner->common.dumpcount); + retval = create_fifo_fds(server, wr->fd_entity.fd, &write_fd, &read_fd, reader.pipe_inner->is_dumping); if (retval < 0) goto cleanup; diff --git a/src/logger/logger_internal.h b/src/logger/logger_internal.h index ba347c9..56545fb 100644 --- a/src/logger/logger_internal.h +++ b/src/logger/logger_internal.h @@ -164,10 +164,7 @@ struct writer { struct reader { struct fd_entity fd_entity_sink; struct fd_entity fd_entity_source; - struct log_file file; dlogutil_filter_options_s* filter; - bool dumpcount; - bool monitor; }; struct reader_pipe { @@ -176,11 +173,15 @@ struct reader_pipe { struct timespec last_read_time; int partial_log_size; char partial_log[sizeof(struct dlogutil_entry_with_msg)]; + struct log_file file; + bool is_dumping; + bool monitor; struct reader common; }; struct reader_logger { log_id_t buf_id; + struct log_file file; struct reader common; }; diff --git a/src/tests/logger.c b/src/tests/logger.c index 0679865..1796602 100644 --- a/src/tests/logger.c +++ b/src/tests/logger.c @@ -442,7 +442,7 @@ log_storage_reader *__wrap_log_storage_new_reader(log_storage *storage, { assert(storage == new_reader_storage_correct); assert(user_data == new_reader_reader_correct); - assert(dumping == new_reader_reader_correct->common.dumpcount); + assert(dumping == new_reader_reader_correct->is_dumping); assert(callback == reader_notify_losing_log); return new_reader_fail ? NULL : (log_storage_reader *)0x600D; @@ -807,9 +807,7 @@ int main() .tv_sec = 0, .tv_nsec = 100000000, }, - .common = { - .file = {}, - }, + .file = {}, }; assert(!reader_is_bufferable(&reader)); @@ -818,7 +816,7 @@ int main() assert(!reader_is_bufferable(&reader)); reader.buf_ptr = NULL; - reader.common.file.path = "/dev/null"; + reader.file.path = "/dev/null"; assert(!reader_is_bufferable(&reader)); reader.buf_ptr = (struct log_buffer *)0x5BU; @@ -869,13 +867,13 @@ int main() struct reader_pipe *reader2 = calloc(1, sizeof(struct reader_pipe)); assert(reader2); - reader2->common.file.fd = 1; + reader2->file.fd = 1; reader_pipe_free(reader2); assert(last_free == reader2 && last_logfile_free_fd == 1); reader2 = calloc(1, sizeof(struct reader_pipe)); assert(reader2); - reader2->common.file.fd = 3; + reader2->file.fd = 3; reader2->common.fd_entity_sink = ent; assert(reader2->common.fd_entity_sink.fd == 2); reader2->common.fd_entity_source.fd = 1; -- 2.7.4