From ca1e03f3ea7517fe5c7cfdbc79782ca7e21e460f Mon Sep 17 00:00:00 2001
From: "Scott E. Graves" <scott.e.graves@protonmail.com>
Date: Thu, 25 Jul 2024 10:46:29 -0500
Subject: [PATCH] logging changes

---
 .../events/consumers/logging_consumer.hpp     | 21 +++++++------
 .../src/events/consumers/logging_consumer.cpp | 30 +++++++++----------
 2 files changed, 26 insertions(+), 25 deletions(-)

diff --git a/repertory/librepertory/include/events/consumers/logging_consumer.hpp b/repertory/librepertory/include/events/consumers/logging_consumer.hpp
index b8db5863..dd084454 100644
--- a/repertory/librepertory/include/events/consumers/logging_consumer.hpp
+++ b/repertory/librepertory/include/events/consumers/logging_consumer.hpp
@@ -29,24 +29,27 @@ class logging_consumer {
   E_CONSUMER();
 
 public:
-  logging_consumer(const std::string &log_directory, const event_level &level);
+  logging_consumer(event_level level, std::string log_directory);
 
   ~logging_consumer();
 
 private:
-  const std::uint8_t MAX_LOG_FILES = 5;
-  const std::uint64_t MAX_LOG_FILE_SIZE = (1024 * 1024 * 5);
+  static constexpr const std::uint8_t MAX_LOG_FILES{5U};
+  static constexpr const std::uint64_t MAX_LOG_FILE_SIZE{1024ULL * 1024ULL *
+                                                         5ULL};
 
 private:
-  event_level event_level_ = event_level::normal;
-  const std::string log_directory_;
-  const std::string log_path_;
-  bool logging_active_ = true;
+  event_level event_level_;
+  std::string log_directory_;
+  std::string log_path_;
+
+private:
+  std::deque<std::shared_ptr<event>> event_queue_;
+  FILE *log_file_{nullptr};
   std::mutex log_mutex_;
   std::condition_variable log_notify_;
-  std::deque<std::shared_ptr<event>> event_queue_;
+  bool logging_active_{true};
   std::unique_ptr<std::thread> logging_thread_;
-  FILE *log_file_ = nullptr;
 
 private:
   void check_log_roll(std::size_t count);
diff --git a/repertory/librepertory/src/events/consumers/logging_consumer.cpp b/repertory/librepertory/src/events/consumers/logging_consumer.cpp
index 5adf21a2..6a4e2b45 100644
--- a/repertory/librepertory/src/events/consumers/logging_consumer.cpp
+++ b/repertory/librepertory/src/events/consumers/logging_consumer.cpp
@@ -30,8 +30,7 @@
 #include "utils/utils.hpp"
 
 namespace repertory {
-logging_consumer::logging_consumer(const std::string &log_directory,
-                                   const event_level &level)
+logging_consumer::logging_consumer(event_level level, std::string log_directory)
     : event_level_(level),
       log_directory_(utils::path::absolute(log_directory)),
       log_path_(utils::path::combine(log_directory, {"repertory.log"})) {
@@ -56,10 +55,10 @@ logging_consumer::logging_consumer(const std::string &log_directory,
 logging_consumer::~logging_consumer() {
   E_CONSUMER_RELEASE();
 
-  unique_mutex_lock l(log_mutex_);
+  unique_mutex_lock lock(log_mutex_);
   logging_active_ = false;
   log_notify_.notify_all();
-  l.unlock();
+  lock.unlock();
 
   logging_thread_->join();
   logging_thread_.reset();
@@ -72,18 +71,19 @@ void logging_consumer::check_log_roll(std::size_t count) {
   constexpr const auto *function_name = static_cast<const char *>(__FUNCTION__);
 
   std::uint64_t file_size{};
-  const auto success = utils::file::get_file_size(log_path_, file_size);
+  auto success = utils::file::get_file_size(log_path_, file_size);
   if (success && (file_size + count) >= MAX_LOG_FILE_SIZE) {
     close_log_file();
     for (std::uint8_t i = MAX_LOG_FILES; i > 0u; i--) {
-      const auto temp_log_path = utils::path::combine(
+      auto temp_log_path = utils::path::combine(
           log_directory_, {"repertory." + std::to_string(i) + ".log"});
       if (utils::file::is_file(temp_log_path)) {
         if (i == MAX_LOG_FILES) {
           if (not utils::file::retry_delete_file(temp_log_path)) {
+            // TODO Handle error
           }
         } else {
-          const auto next_file_path = utils::path::combine(
+          auto next_file_path = utils::path::combine(
               log_directory_,
               {"repertory." + std::to_string(i + std::uint8_t(1)) + ".log"});
           if (not utils::file::move_file(temp_log_path, next_file_path)) {
@@ -132,7 +132,7 @@ void logging_consumer::logging_thread(bool drain) {
       events.pop_front();
 
       if (event->get_event_level() <= event_level_) {
-        const std::string msg = ([&]() -> std::string {
+        auto msg = ([&]() -> std::string {
           struct tm local_time {};
           utils::get_local_time_now(local_time);
 
@@ -146,7 +146,7 @@ void logging_consumer::logging_thread(bool drain) {
         check_log_roll(msg.length());
         auto retry = true;
         for (int i = 0; retry && (i < 2); i++) {
-          retry = (not log_file_ || (fwrite(&msg[0], 1, msg.length(),
+          retry = (not log_file_ || (fwrite(msg.data(), 1, msg.length(),
                                             log_file_) != msg.length()));
           if (retry) {
             reopen_log_file();
@@ -162,19 +162,17 @@ void logging_consumer::logging_thread(bool drain) {
 }
 
 void logging_consumer::process_event(const event &event) {
-  {
-    mutex_lock l(log_mutex_);
-    event_queue_.push_back(event.clone());
-    log_notify_.notify_all();
-  }
+  mutex_lock lock(log_mutex_);
+  event_queue_.push_back(event.clone());
+  log_notify_.notify_all();
 }
 
 void logging_consumer::reopen_log_file() {
   close_log_file();
 #if defined(_WIN32)
-  log_file_ = _fsopen(&log_path_[0], "a+", _SH_DENYWR);
+  log_file_ = _fsopen(log_path_.c_str(), "a+", _SH_DENYWR);
 #else
-  log_file_ = fopen(&log_path_[0], "a+");
+  log_file_ = fopen(log_path_.c_str(), "a+");
 #endif
 }
 } // namespace repertory