From e32ba0291f54764c0a38b56aad14cceb980b2be5 Mon Sep 17 00:00:00 2001 From: Michal Bloch Date: Fri, 9 Dec 2022 16:29:01 +0100 Subject: [PATCH] dlog_logger: refactoring Now there's a function that just handles system IO, and another function that just parses data. This is to make adapting the IO part to multi-threading easier. Change-Id: Ia185928c092de94e617ec76a937e312d90cf7b3e Signed-off-by: Michal Bloch --- src/logger/reader_logger.c | 82 ++++++++++++++++++++++++++-------------------- 1 file changed, 46 insertions(+), 36 deletions(-) diff --git a/src/logger/reader_logger.c b/src/logger/reader_logger.c index e11f8f5..d061387 100644 --- a/src/logger/reader_logger.c +++ b/src/logger/reader_logger.c @@ -8,6 +8,50 @@ static void free_reader_logger(struct reader_common *reader) * does everything via sub-readers. */ } +static void handle_single_log(struct reader_logger *reader, struct now_t time, int read_count) +{ + assert(reader); + assert(read_count > 0); + + if (reader->skip_count > 0) { + reader->skip_count -= read_count; + return; + } + + struct android_logger_entry *const ale = (struct android_logger_entry *) reader->read_buffer; + + if (read_count < sizeof *ale) { + /* This should not happen, but don't treat this as a hard + * invariant because kernel bugs happen. Not salvageable. */ + return; + } else if (read_count == sizeof *ale) { + if (ale->len != 0) { + /* Invalid entry. Likely a kernel bug. + * At any rate, not salvageable. */ + return; + } else { + /* A "valid" empty entry. Is probably + * salvageable (so that the information + * that a program tried to log something + * and the relevant timestamp are not + * completely lost). Maybe later though. */ + return; + } + } + + reader->read_buffer[read_count] = '\0'; + + struct dlogutil_entry_with_msg entry; + parse_androidlogger_message(ale, &entry.header, read_count); + add_recv_timestamp(&entry.header, time); + + // TODO: Consider calling this in a more robust way (and not having pipe in the name) + fixup_pipe_msg(&entry, read_count - sizeof(*ale)); + + // Ignore intermittent write failures. Not a reason to remove the reader. + (void) reader_apply_log_to_subs(&reader->common, &entry.header); +} + /** * @brief Service reader file * @details Handles readers reading directly from file @@ -20,7 +64,6 @@ static int service_reader_logger(struct reader_common *_reader, struct now_t tim { struct reader_logger *const reader = (struct reader_logger *) _reader; assert(reader); - struct android_logger_entry *const ale = (struct android_logger_entry *) reader->read_buffer; /* The devices for the Android Logger only return one log per read(). * So using an 'if' here would be wasteful and, more importantly, too slow in the case where other logs come in. @@ -39,42 +82,9 @@ static int service_reader_logger(struct reader_common *_reader, struct now_t tim break; else return -errno; - } else { - if (reader->skip_count > 0) { - reader->skip_count -= r; - continue; - } - - if (r < sizeof *ale) { - /* This should not happen, but don't treat this as a hard - * invariant because kernel bugs happen. Not salvageable. */ - continue; - } else if (r == sizeof *ale) { - if (ale->len != 0) { - /* Invalid entry. Likely a kernel bug. - * At any rate, not salvageable. */ - continue; - } else { - /* A "valid" empty entry. Is probably - * salvageable (so that the information - * that a program tried to log something - * and the relevant timestamp are not - * completely lost). Maybe later though. */ - continue; - } - } - reader->read_buffer[r] = '\0'; - - struct dlogutil_entry_with_msg entry; - parse_androidlogger_message(ale, &entry.header, r); - add_recv_timestamp(&entry.header, time); - - // TODO: Consider calling this in a more robust way (and not having pipe in the name) - fixup_pipe_msg(&entry, r - sizeof(*ale)); - - // Ignore intermittent write failures. Not a reason to remove the reader. - (void) reader_apply_log_to_subs(&reader->common, &entry.header); } + + handle_single_log(reader, time, r); } return 0; -- 2.7.4