From ef6d1aeaaaa6c5df63d44033852e1eb683130414 Mon Sep 17 00:00:00 2001 From: Dean Lee Date: Tue, 2 Feb 2021 13:00:42 +0800 Subject: [PATCH] Logger: new class BZFile (#19959) * add class BZFile * inline * cleanup includes * stack bzfile * log first error * remove assert * rename log_err to err_logged * assert in destructor * LOGE * don't assert statement * assert error of fclose --- selfdrive/loggerd/bootlog.cc | 29 ++-------------- selfdrive/loggerd/logger.cc | 65 ++++-------------------------------- selfdrive/loggerd/logger.h | 44 ++++++++++++++++++++---- 3 files changed, 48 insertions(+), 90 deletions(-) diff --git a/selfdrive/loggerd/bootlog.cc b/selfdrive/loggerd/bootlog.cc index 3f01b951..8be21f3a 100644 --- a/selfdrive/loggerd/bootlog.cc +++ b/selfdrive/loggerd/bootlog.cc @@ -1,12 +1,6 @@ #include #include -#include - -#include - #include "common/swaglog.h" -#include "common/util.h" -#include "messaging.hpp" #include "logger.h" int main(int argc, char** argv) { @@ -26,30 +20,13 @@ int main(int argc, char** argv) { int r = logger_mkpath((char*)path.c_str()); assert(r == 0); - FILE * file = fopen(path.c_str(), "wb"); - assert(file != nullptr); - - // Open as bz2 - int bzerror; - BZFILE* bz_file = BZ2_bzWriteOpen(&bzerror, file, 9, 0, 30); - assert(bzerror == BZ_OK); + BZFile bz_file(path.c_str()); // Write initdata - kj::Array init_msg = logger_build_init_data(); - auto bytes = init_msg.asBytes(); - BZ2_bzWrite(&bzerror, bz_file, bytes.begin(), bytes.size()); - assert(bzerror == BZ_OK); + bz_file.write(logger_build_init_data().asBytes()); // Write bootlog - kj::Array boot_msg = logger_build_boot(); - bytes = boot_msg.asBytes(); - BZ2_bzWrite(&bzerror, bz_file, bytes.begin(), bytes.size()); - assert(bzerror == BZ_OK); + bz_file.write(logger_build_boot().asBytes()); - // Close bz2 and file - BZ2_bzWriteClose(&bzerror, bz_file, 0, NULL, NULL); - assert(bzerror == BZ_OK); - - fclose(file); return 0; } diff --git a/selfdrive/loggerd/logger.cc b/selfdrive/loggerd/logger.cc index a61881c7..a48ebe5b 100644 --- a/selfdrive/loggerd/logger.cc +++ b/selfdrive/loggerd/logger.cc @@ -9,8 +9,6 @@ #include #include -#include -#include #include #include #include @@ -20,7 +18,6 @@ #include "common/swaglog.h" #include "common/params.h" -#include "common/util.h" #include "common/version.h" #include "messaging.hpp" #include "logger.h" @@ -200,46 +197,14 @@ static LoggerHandle* logger_open(LoggerState *s, const char* root_path) { if (lock_file == NULL) return NULL; fclose(lock_file); - h->log_file = fopen(h->log_path, "wb"); - if (h->log_file == NULL) goto fail; - + h->log = std::make_unique(h->log_path); if (s->has_qlog) { - h->qlog_file = fopen(h->qlog_path, "wb"); - if (h->qlog_file == NULL) goto fail; - } - - int bzerror; - h->bz_file = BZ2_bzWriteOpen(&bzerror, h->log_file, 9, 0, 30); - if (bzerror != BZ_OK) goto fail; - - if (s->has_qlog) { - h->bz_qlog = BZ2_bzWriteOpen(&bzerror, h->qlog_file, 9, 0, 30); - if (bzerror != BZ_OK) goto fail; + h->q_log = std::make_unique(h->qlog_path); } pthread_mutex_init(&h->lock, NULL); h->refcnt++; return h; - -fail: - LOGE("logger failed to open files"); - if (h->bz_file) { - BZ2_bzWriteClose(&bzerror, h->bz_file, 0, NULL, NULL); - h->bz_file = NULL; - } - if (h->bz_qlog) { - BZ2_bzWriteClose(&bzerror, h->bz_qlog, 0, NULL, NULL); - h->bz_qlog = NULL; - } - if (h->qlog_file) { - fclose(h->qlog_file); - h->qlog_file = NULL; - } - if (h->log_file) { - fclose(h->log_file); - h->log_file = NULL; - } - return NULL; } int logger_next(LoggerState *s, const char* root_path, @@ -310,11 +275,9 @@ void logger_close(LoggerState *s) { void lh_log(LoggerHandle* h, uint8_t* data, size_t data_size, bool in_qlog) { pthread_mutex_lock(&h->lock); assert(h->refcnt > 0); - int bzerror; - BZ2_bzWrite(&bzerror, h->bz_file, data, data_size); - - if (in_qlog && h->bz_qlog != NULL) { - BZ2_bzWrite(&bzerror, h->bz_qlog, data, data_size); + h->log->write(data, data_size); + if (in_qlog && h->q_log) { + h->q_log->write(data, data_size); } pthread_mutex_unlock(&h->lock); } @@ -324,22 +287,8 @@ void lh_close(LoggerHandle* h) { assert(h->refcnt > 0); h->refcnt--; if (h->refcnt == 0) { - if (h->bz_file){ - int bzerror; - BZ2_bzWriteClose(&bzerror, h->bz_file, 0, NULL, NULL); - h->bz_file = NULL; - } - if (h->bz_qlog){ - int bzerror; - BZ2_bzWriteClose(&bzerror, h->bz_qlog, 0, NULL, NULL); - h->bz_qlog = NULL; - } - if (h->qlog_file) { - fclose(h->qlog_file); - h->qlog_file = NULL; - } - fclose(h->log_file); - h->log_file = NULL; + h->log.reset(nullptr); + h->q_log.reset(nullptr); unlink(h->lock_path); pthread_mutex_unlock(&h->lock); pthread_mutex_destroy(&h->lock); diff --git a/selfdrive/loggerd/logger.h b/selfdrive/loggerd/logger.h index f3555866..78c2fc04 100644 --- a/selfdrive/loggerd/logger.h +++ b/selfdrive/loggerd/logger.h @@ -3,9 +3,11 @@ #include #include #include +#include #include #include #include +#include "common/util.h" #if defined(QCOM) || defined(QCOM2) const std::string LOG_ROOT = "/data/media/0/realdata"; @@ -15,18 +17,48 @@ const std::string LOG_ROOT = util::getenv_default("HOME", "/.comma/media/0/reald #define LOGGER_MAX_HANDLES 16 +class BZFile { + public: + BZFile(const char* path) { + file = fopen(path, "wb"); + assert(file != nullptr); + int bzerror; + bz_file = BZ2_bzWriteOpen(&bzerror, file, 9, 0, 30); + assert(bzerror == BZ_OK); + } + ~BZFile() { + int bzerror; + BZ2_bzWriteClose(&bzerror, bz_file, 0, nullptr, nullptr); + if (bzerror != BZ_OK) { + LOGE("BZ2_bzWriteClose error, bzerror=%d", bzerror); + } + int err = fclose(file); + assert(err == 0); + } + inline void write(void* data, size_t size) { + int bzerror; + BZ2_bzWrite(&bzerror, bz_file, data, size); + if (bzerror != BZ_OK && !error_logged) { + LOGE("BZ2_bzWrite error, bzerror=%d", bzerror); + error_logged = true; + } + } + inline void write(kj::ArrayPtr array) { write(array.begin(), array.size()); } + + private: + bool error_logged = false; + FILE* file = nullptr; + BZFILE* bz_file = nullptr; +}; + typedef struct LoggerHandle { pthread_mutex_t lock; int refcnt; char segment_path[4096]; char log_path[4096]; - char lock_path[4096]; - FILE* log_file; - BZFILE* bz_file; - - FILE* qlog_file; char qlog_path[4096]; - BZFILE* bz_qlog; + char lock_path[4096]; + std::unique_ptr log, q_log; } LoggerHandle; typedef struct LoggerState {