codebase review and improvements

Author: odygrd

CHANGELOG.md

@@ -214,6 +214,7 @@
   `3.12`. ([#686](https://github.com/odygrd/quill/issues/686))
 - Correct the installation location of pkg-config files. They are now properly placed in `/usr/local/lib`.
   ([#715](https://github.com/odygrd/quill/issues/715))
+- Removed deprecated `TriviallyCopyableTypeCodec`
 
 ## v8.2.0
 

CMakeLists.txt

@@ -254,7 +254,6 @@ set(HEADER_FILES
         include/quill/LogMacros.h
         include/quill/StopWatch.h
         include/quill/StringRef.h
-        include/quill/TriviallyCopyableCodec.h
         include/quill/UserClockSource.h
         include/quill/Utility.h
 )

include/quill/Backend.h

@@ -1,6 +1,6 @@
 /**
  * @page copyright
- * Copyright(c) 2020-present, Odysseas Georgoudis & quill contributors.* @page copyright
+ * Copyright(c) 2020-present, Odysseas Georgoudis & quill contributors.
  * Distributed under the MIT License (http://opensource.org/licenses/MIT)
  */
 

include/quill/CsvWriter.h

@@ -183,7 +183,6 @@ class CsvWriter
    */
   ~CsvWriter()
   {
-    _logger->flush_log();
     frontend_t::remove_logger(_logger);
   }
 

include/quill/Logger.h

@@ -115,10 +115,15 @@ class LoggerImpl : public detail::LoggerBase
     {
       current_timestamp = detail::get_timestamp_ns<std::chrono::system_clock>();
     }
-    else
+    else if (user_clock)
     {
       current_timestamp = user_clock->now();
     }
+    else
+    {
+      // not expected
+      current_timestamp = 0;
+    }
 
     if (QUILL_UNLIKELY(thread_context == nullptr))
     {
@@ -359,15 +364,7 @@ class LoggerImpl : public detail::LoggerBase
    */
   QUILL_NODISCARD QUILL_ATTRIBUTE_HOT std::byte* _prepare_write_buffer(size_t total_size)
   {
-    if constexpr (using_unbounded_queue)
-    {
-      // MSVC doesn't like the template keyword, but every other compiler requires it
-      return thread_context->get_spsc_queue<frontend_options_t::queue_type>().prepare_write(total_size);
-    }
-    else
-    {
-      return thread_context->get_spsc_queue<frontend_options_t::queue_type>().prepare_write(total_size);
-    }
+    return thread_context->get_spsc_queue<frontend_options_t::queue_type>().prepare_write(total_size);
   }
 };
 

include/quill/StopWatch.h

@@ -70,16 +70,7 @@ class StopWatch
    */
   [[nodiscard]] std::chrono::duration<double> elapsed() const
   {
-    if constexpr (ClockType == ClockSourceType::Tsc)
-    {
-      return std::chrono::duration<double>(std::chrono::nanoseconds{
-        static_cast<uint64_t>(static_cast<double>(rdtsc() - _start_tp) * _ns_per_tick)});
-    }
-    else
-    {
-      return std::chrono::duration<double>(std::chrono::nanoseconds{
-        detail::get_timestamp_ns<std::chrono::steady_clock>() - _start_tp});
-    }
+    return elapsed_as<std::chrono::duration<double>>();
   }
 
   /**

include/quill/StringRef.h

@@ -34,7 +34,8 @@ class StringRef
 public:
   explicit StringRef(std::string const& str) : _str_view(str) {};
   explicit StringRef(std::string_view str) : _str_view(str) {};
-  explicit StringRef(char const* str) : _str_view(str, strlen(str)) {};
+  explicit StringRef(char const* str)
+    : _str_view(str, detail::safe_strnlen(str, std::numeric_limits<uint32_t>::max())) {};
   StringRef(char const* str, size_t size) : _str_view(str, size) {};
 
   QUILL_NODISCARD std::string_view const& get_string_view() const noexcept { return _str_view; }

include/quill/TriviallyCopyableCodec.h

@@ -33,6 +33,7 @@ class UserClockSource
 
   /**
    * Returns time since epoch in nanoseconds
+   * @return The current timestamp in nanoseconds
    */
   QUILL_NODISCARD QUILL_ATTRIBUTE_HOT virtual uint64_t now() const = 0;
 };

include/quill/UserClockSource.h

@@ -26,27 +26,31 @@ namespace utility
  * @return A string containing the hexadecimal representation of the given buffer.
  */
 template <typename T>
-QUILL_NODISCARD std::string to_hex(T* buffer, size_t size) noexcept
+QUILL_NODISCARD std::string to_hex(T const* buffer, size_t size)
 {
   static constexpr char hex_chars[] = "0123456789ABCDEF";
 
+  if (!buffer || size == 0)
+  {
+    return {};
+  }
+
+  // Each byte needs 2 hex chars, and all but the last one need spaces
   std::string hex_string;
-  hex_string.reserve(3 * size);
+  hex_string.reserve(size > 0 ? (3 * size - 1) : 0);
 
   for (size_t i = 0; i < size; ++i)
   {
-    // 00001111 mask
-    static constexpr uint8_t mask = 0x0Fu;
-
-    // add the first four bits
-    hex_string += hex_chars[(buffer[i] >> 4u) & mask];
+    const auto byte = static_cast<uint8_t>(buffer[i]);
 
-    // add the remaining bits
-    hex_string += hex_chars[buffer[i] & mask];
+    // Add the first four bits
+    hex_string += hex_chars[(byte >> 4) & 0x0F];
+    // Add the remaining bits
+    hex_string += hex_chars[byte & 0x0F];
 
+    // Add a space delimiter after all but the last byte
     if (i != (size - 1))
     {
-      // add a space delimiter
       hex_string += ' ';
     }
   }

include/quill/Utility.h

@@ -90,7 +90,7 @@ class BackendManager
   }
 
   /***/
-  QUILL_NODISCARD uint64_t convert_rdtsc_to_epoch_time(uint64_t rdtsc_value) const noexcept
+  QUILL_NODISCARD uint64_t convert_rdtsc_to_epoch_time(uint64_t rdtsc_value) const
   {
     return _backend_worker.time_since_epoch(rdtsc_value);
   }

include/quill/backend/BackendManager.h

@@ -98,7 +98,7 @@ class BackendWorker
   /***/
   QUILL_NODISCARD bool is_running() const noexcept
   {
-    return _is_worker_running.load(std::memory_order_relaxed);
+    return _is_worker_running.load(std::memory_order_acquire);
   }
 
   /**
@@ -140,7 +140,7 @@ class BackendWorker
     }
 
     std::thread worker(
-      [this, &options]()
+      [this, options]()
       {
         _init(options);
 

include/quill/backend/BackendWorker.h

@@ -378,7 +378,7 @@ class StringFromTime
   /***/
   QUILL_NODISCARD static time_t _nearest_quarter_hour_timestamp(time_t timestamp) noexcept
   {
-    time_t const nearest_quarter_hour_ts = timestamp - (timestamp % 900);
+    time_t const nearest_quarter_hour_ts = (timestamp / 900) * 900;
     return nearest_quarter_hour_ts;
   }
 

include/quill/backend/StringFromTime.h

@@ -32,7 +32,8 @@ class TransitEventBuffer
 
   // Move constructor
   TransitEventBuffer(TransitEventBuffer&& other) noexcept
-    : _capacity(other._capacity),
+    : _initial_capacity(other._initial_capacity),
+      _capacity(other._capacity),
       _storage(std::move(other._storage)),
       _mask(other._mask),
       _reader_pos(other._reader_pos),
@@ -49,6 +50,7 @@ class TransitEventBuffer
   {
     if (this != &other)
     {
+      _initial_capacity = other._initial_capacity;
       _capacity = other._capacity;
       _storage = std::move(other._storage);
       _mask = other._mask;
@@ -152,4 +154,4 @@ class TransitEventBuffer
 
 } // namespace detail
 
-QUILL_END_NAMESPACE
+QUILL_END_NAMESPACE