Refactor logging and statistics based on ISO C++ Core Guidelines. More specifically:

  - Avoid C variable args, use variadic templates instead.
  - Avoid raw pointers.
  - Avoid C arrays.
  - Use scoped enums

Signed-off-by: Mulugeta Mammo <[email protected]>

Author: mulugetam

common.cmake

@@ -71,7 +71,7 @@ if(TSAN)
   set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=thread")
 endif()
 
-set(CMAKE_CXX_STANDARD 17)
+set(CMAKE_CXX_STANDARD 20)
 set(CMAKE_CXX_STANDARD_REQUIRED True)
 
 if(USE_IAA)

config/config.cpp

@@ -50,7 +50,7 @@ bool LoadConfigFile(std::string& file_content, const char* file_path) {
     "iaa_prepend_empty_block",
     "qat_periodical_polling",
     "qat_compression_level",
-	"qat_compression_allow_chunking",
+	  "qat_compression_allow_chunking",
     "log_level",
     "log_stats_samples"
   };

config/config_reader.cpp

@@ -26,7 +26,7 @@ bool ConfigReader::GetValue(std::string tag, uint32_t& value,
       ret_val = true;
     } else {
       Log(LogLevel::LOG_INFO,
-          "ConfigReader::GetValue  Line %d invalid input value for tag %s\n",
+          "ConfigReader::GetValue  Line {} invalid input value for tag {}\n",
           __LINE__, tag.c_str());
       value = 0;
       ret_val = false;
@@ -43,7 +43,7 @@ bool ConfigReader::GetValue(std::string tag, std::string& value) {
     value = it->second;
     if ((tag == "log_file") && !IsValidFileNameOrPath(value)) {
       Log(LogLevel::LOG_INFO,
-          "ConfigReader::GetValue  Line %d invalid log_file value  %s\n",
+          "ConfigReader::GetValue  Line {} invalid log_file value  {}\n",
           __LINE__, value.c_str());
       value = "";
       ret_val = false;

iaa.cpp

@@ -56,7 +56,7 @@ uint32_t GetFormatFlag(int window_bits) {
 int CompressIAA(uint8_t* input, uint32_t* input_length, uint8_t* output,
                 uint32_t* output_length, qpl_path_t execution_path,
                 int window_bits, uint32_t max_compressed_size, bool gzip_ext) {
-  Log(LogLevel::LOG_INFO, "CompressIAA() Line %d input_length %d\n", __LINE__,
+  Log(LogLevel::LOG_INFO, "CompressIAA() Line {} input_length {}\n", __LINE__,
       *input_length);
 
   // State from previous job execution not ignored/reset correctly for zlib
@@ -68,7 +68,7 @@ int CompressIAA(uint8_t* input, uint32_t* input_length, uint8_t* output,
 
   qpl_job* job = job_.GetJob(execution_path);
   if (job == nullptr) {
-    Log(LogLevel::LOG_ERROR, "CompressIAA() Line %d Error qpl_job is null\n",
+    Log(LogLevel::LOG_ERROR, "CompressIAA() Line {} Error qpl_job is null\n",
         __LINE__);
     return 1;
   }
@@ -113,7 +113,7 @@ int CompressIAA(uint8_t* input, uint32_t* input_length, uint8_t* output,
 
   qpl_status status = qpl_execute_job(job);
   if (status != QPL_STS_OK) {
-    Log(LogLevel::LOG_ERROR, "CompressIAA() Line %d status %d\n", __LINE__,
+    Log(LogLevel::LOG_ERROR, "CompressIAA() Line {} status {}\n", __LINE__,
         status);
     return 1;
   }
@@ -127,7 +127,7 @@ int CompressIAA(uint8_t* input, uint32_t* input_length, uint8_t* output,
   *input_length = job->total_in;
   *output_length = job->total_out;
 
-  Log(LogLevel::LOG_INFO, "CompressIAA() Line %d compressed_size %d\n",
+  Log(LogLevel::LOG_INFO, "CompressIAA() Line {} compressed_size {}\n",
       __LINE__, *output_length);
 
   if (output_shift > 0) {
@@ -180,7 +180,7 @@ int CompressIAA(uint8_t* input, uint32_t* input_length, uint8_t* output,
 int UncompressIAA(uint8_t* input, uint32_t* input_length, uint8_t* output,
                   uint32_t* output_length, qpl_path_t execution_path,
                   int window_bits, bool* end_of_stream, bool detect_gzip_ext) {
-  Log(LogLevel::LOG_INFO, "UncompressIAA() Line %d input_length %d\n", __LINE__,
+  Log(LogLevel::LOG_INFO, "UncompressIAA() Line {} input_length {}\n", __LINE__,
       *input_length);
 
   bool gzip_ext = false;
@@ -197,7 +197,7 @@ int UncompressIAA(uint8_t* input, uint32_t* input_length, uint8_t* output,
 
   qpl_job* job = job_.GetJob(execution_path);
   if (job == nullptr) {
-    Log(LogLevel::LOG_ERROR, "UncompressIAA() Line %d Error qpl_job is null\n",
+    Log(LogLevel::LOG_ERROR, "UncompressIAA() Line {} Error qpl_job is null\n",
         __LINE__);
     return 1;
   }
@@ -218,7 +218,7 @@ int UncompressIAA(uint8_t* input, uint32_t* input_length, uint8_t* output,
   qpl_status status = qpl_execute_job(job);
   if (status != QPL_STS_OK) {
     Log(LogLevel::LOG_ERROR,
-        "UncompressIAA() Line %d qpl_execute_job status %d\n", __LINE__,
+        "UncompressIAA() Line {} qpl_execute_job status {}\n", __LINE__,
         status);
     return 1;
   }
@@ -230,7 +230,7 @@ int UncompressIAA(uint8_t* input, uint32_t* input_length, uint8_t* output,
     *input_length = gzip_ext_dest_size + GZIP_EXT_HDRFTR_SIZE;
   }
   *end_of_stream = true;
-  Log(LogLevel::LOG_INFO, "UncompressIAA() Line %d output size %d\n", __LINE__,
+  Log(LogLevel::LOG_INFO, "UncompressIAA() Line {} output size {}\n", __LINE__,
       job->total_out);
   return 0;
 }
@@ -242,7 +242,7 @@ bool SupportedOptionsIAA(int window_bits, uint32_t input_length,
       (window_bits >= 24 && window_bits <= 31)) {
     if (input_length > MAX_BUFFER_SIZE || output_length > MAX_BUFFER_SIZE) {
       Log(LogLevel::LOG_INFO,
-          "SupportedOptionsIAA() Line %d input length %d or output length %d "
+          "SupportedOptionsIAA() Line {} input length {} or output length {} "
           "is more than 2MB\n",
           __LINE__, input_length, output_length);
       return false;
@@ -263,7 +263,7 @@ bool PrependedEmptyBlockPresent(uint8_t* input, uint32_t input_length,
       input[header_length + 2] == 0 && input[header_length + 3] == 0xFF &&
       input[header_length + 4] == 0xFF) {
     Log(LogLevel::LOG_INFO,
-        "PrependedEmptyBlockPresent() Line %d Empty block detected\n",
+        "PrependedEmptyBlockPresent() Line {} Empty block detected\n",
         __LINE__);
     return true;
   }
@@ -276,7 +276,7 @@ bool IsIAADecompressible(uint8_t* input, uint32_t input_length,
   CompressedFormat format = GetCompressedFormat(window_bits);
   if (format == CompressedFormat::ZLIB) {
     int window = GetWindowSizeFromZlibHeader(input, input_length);
-    Log(LogLevel::LOG_INFO, "IsIAADecompressible() Line %d window %d\n",
+    Log(LogLevel::LOG_INFO, "IsIAADecompressible() Line {} window {}\n",
         __LINE__, window);
     return window <= 12;
   } else {

logging.h

@@ -3,90 +3,92 @@
 
 #pragma once
 
-#include <errno.h>
-#include <fcntl.h>
-#include <stdarg.h>
-#include <stdio.h>
-#include <stdlib.h>
-#include <string.h>
-#include <unistd.h>
+#include <format>
+#include <fstream>
+#include <iostream>
+#include <memory>
+#include <mutex>
+#include <sstream>
+#include <utility>
 
 #include "config/config.h"
 #include "utils.h"
+
 using namespace config;
+
 enum class LogLevel { LOG_NONE = 0, LOG_INFO = 1, LOG_ERROR = 2 };
 
 #if defined(DEBUG_LOG) || defined(ENABLE_STATISTICS)
-inline FILE* log_file_stream = nullptr;
+inline std::unique_ptr<std::ofstream> log_file_stream = nullptr;
 
-inline static void CreateLogFile(const char* file_name) {
-  log_file_stream = fopen(file_name, "a");
+inline void CreateLogFile(const char* file_name) {
+  log_file_stream = std::make_unique<std::ofstream>(file_name, std::ios::app);
 }
 
-inline static void CloseLogFile() {
-  if (log_file_stream != nullptr) {
-    fclose(log_file_stream);
+inline void CloseLogFile() { log_file_stream.reset(); }
+
+inline std::ostream& GetLogStream() {
+  if (log_file_stream && log_file_stream->is_open()) {
+    return *log_file_stream;
   }
+  return std::cout;
 }
+
 #endif
 
 #ifdef DEBUG_LOG
-static inline void Log(LogLevel level, const char* format, ...) {
+
+static std::mutex log_mutex;
+
+template <typename... Args>
+inline void Log(LogLevel level, const char* fmt, Args&&... args) {
+  std::lock_guard<std::mutex> lock(log_mutex);
+
   if (static_cast<uint32_t>(level) < configs[LOG_LEVEL]) {
     return;
   }
-
-  FILE* stream = stdout;
-  if (log_file_stream != nullptr) {
-    stream = log_file_stream;
-  }
-
+  auto& stream = GetLogStream();
   switch (level) {
     case LogLevel::LOG_ERROR:
-      fprintf(stream, "Error: ");
+      stream << "Error: ";
       break;
     case LogLevel::LOG_INFO:
-      fprintf(stream, "Info: ");
+      stream << "Info: ";
       break;
     case LogLevel::LOG_NONE:
       return;
   }
-  va_list args;
-  va_start(args, format);
-  vfprintf(stream, format, args);
-  va_end(args);
+  stream << std::vformat(fmt, std::make_format_args(args...));
 }
 #else
 #define Log(...)
 #endif
 
 #ifdef ENABLE_STATISTICS
-static inline void LogStats(const char* stats_str) {
-  FILE* stream = stdout;
-  if (log_file_stream != nullptr) {
-    stream = log_file_stream;
-  }
-
-  fprintf(stream, "Stats:\n");
-  fprintf(stream, "%s", stats_str);
+template <typename... Args>
+inline void LogStats(const char* fmt, Args&&... args) {
+  auto& stream = GetLogStream();
+  stream << "Stats:\n";
+  stream << std::vformat(fmt, std::make_format_args(args...));
 }
 #else
 #define LogStats(...)
 #endif
 
 #ifdef DEBUG_LOG
-static inline void PrintDeflateBlockHeader(LogLevel level, uint8_t* data,
-                                           uint32_t len, int window_bits) {
+template <typename... Args>
+inline void PrintDeflateBlockHeader(LogLevel level, uint8_t* data, uint32_t len,
+                                    int window_bits, Args&&... args) {
   if (static_cast<uint32_t>(level) < configs[LOG_LEVEL]) {
     return;
   }
-
   CompressedFormat format = GetCompressedFormat(window_bits);
   uint32_t header_length = GetHeaderLength(format);
   if (len >= (header_length + 1)) {
-    Log(level, "Deflate block header bfinal=%d, btype=%d\n",
-        data[header_length] & 0b00000001,
-        (data[header_length] & 0b00000110) >> 1);
+    Log(level, "Deflate block header bfinal = {}, btype = {}\n",
+        static_cast<int>(data[header_length] & 0b00000001),
+        static_cast<int>((data[header_length] & 0b00000110) >> 1),
+        std::forward<Args>(args)...);
   }
 }
 #else