Skip to content

Commit 74d7f48

Browse files
mulugetammulugetam
committed
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]>
1 parent 0026971 commit 74d7f48

File tree

4 files changed

+127
-130
lines changed

4 files changed

+127
-130
lines changed

‎logging.h‎

Lines changed: 49 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -3,92 +3,98 @@
33

44
#pragma once
55

6-
#include <errno.h>
7-
#include <fcntl.h>
8-
#include <stdarg.h>
9-
#include <stdio.h>
10-
#include <stdlib.h>
11-
#include <string.h>
12-
#include <unistd.h>
6+
#include <fstream>
7+
#include <iostream>
8+
#include <memory>
9+
#include <sstream>
10+
#include <utility>
1311

1412
#include "config/config.h"
1513
#include "utils.h"
14+
1615
using namespace config;
16+
1717
enum class LogLevel { LOG_NONE = 0, LOG_INFO = 1, LOG_ERROR = 2 };
1818

1919
#if defined(DEBUG_LOG) || defined(ENABLE_STATISTICS)
20-
inline FILE* log_file_stream = nullptr;
20+
inline std::unique_ptr<std::ofstream> log_file_stream = nullptr;
2121

22-
inline static void CreateLogFile(const char* file_name) {
23-
log_file_stream = fopen(file_name, "a");
22+
inline void CreateLogFile(const char* file_name) {
23+
log_file_stream = std::make_unique<std::ofstream>(file_name, std::ios::app);
2424
}
2525

26-
inline static void CloseLogFile() {
27-
if (log_file_stream != nullptr) {
28-
fclose(log_file_stream);
26+
inline void CloseLogFile() { log_file_stream.reset(); }
27+
28+
inline std::ostream& GetLogStream() {
29+
if (log_file_stream && log_file_stream->is_open()) {
30+
return *log_file_stream;
2931
}
32+
return std::cout;
33+
}
34+
35+
inline void LogImpl(std::ostream& stream) { stream << std::endl; }
36+
37+
template <typename T, typename... Args>
38+
inline void LogImpl(std::ostream& stream, T&& first, Args&&... rest) {
39+
stream << std::forward<T>(first);
40+
LogImpl(stream, std::forward<Args>(rest)...);
3041
}
3142
#endif
3243

3344
#ifdef DEBUG_LOG
34-
static inline void Log(LogLevel level, const char* format, ...) {
45+
template <typename... Args>
46+
inline void Log(LogLevel level, Args&&... args) {
3547
if (static_cast<uint32_t>(level) < configs[LOG_LEVEL]) {
3648
return;
3749
}
38-
39-
FILE* stream = stdout;
40-
if (log_file_stream != nullptr) {
41-
stream = log_file_stream;
42-
}
43-
50+
auto& stream = GetLogStream();
4451
switch (level) {
4552
case LogLevel::LOG_ERROR:
46-
fprintf(stream, "Error: ");
53+
stream << "Error: ";
4754
break;
4855
case LogLevel::LOG_INFO:
49-
fprintf(stream, "Info: ");
56+
stream << "Info: ";
5057
break;
5158
case LogLevel::LOG_NONE:
5259
return;
5360
}
54-
va_list args;
55-
va_start(args, format);
56-
vfprintf(stream, format, args);
57-
va_end(args);
61+
LogImpl(stream, std::forward<Args>(args)...);
5862
}
5963
#else
60-
#define Log(...)
64+
template <typename... Args>
65+
inline void Log(LogLevel, Args&&...) {}
6166
#endif
6267

6368
#ifdef ENABLE_STATISTICS
64-
static inline void LogStats(const char* stats_str) {
65-
FILE* stream = stdout;
66-
if (log_file_stream != nullptr) {
67-
stream = log_file_stream;
68-
}
69-
70-
fprintf(stream, "Stats:\n");
71-
fprintf(stream, "%s", stats_str);
69+
template <typename... Args>
70+
inline void LogStats(Args&&... args) {
71+
auto& stream = GetLogStream();
72+
stream << "Stats:\n";
73+
LogImpl(stream, std::forward<Args>(args)...);
7274
}
7375
#else
74-
#define LogStats(...)
76+
template <typename... Args>
77+
inline void LogStats(Args&&...) {}
7578
#endif
7679

7780
#ifdef DEBUG_LOG
78-
static inline void PrintDeflateBlockHeader(LogLevel level, uint8_t* data,
79-
uint32_t len, int window_bits) {
81+
template <typename... Args>
82+
inline void PrintDeflateBlockHeader(LogLevel level, uint8_t* data, uint32_t len,
83+
int window_bits, Args&&... args) {
8084
if (static_cast<uint32_t>(level) < configs[LOG_LEVEL]) {
8185
return;
8286
}
83-
8487
CompressedFormat format = GetCompressedFormat(window_bits);
8588
uint32_t header_length = GetHeaderLength(format);
8689
if (len >= (header_length + 1)) {
87-
Log(level, "Deflate block header bfinal=%d, btype=%d\n",
88-
data[header_length] & 0b00000001,
89-
(data[header_length] & 0b00000110) >> 1);
90+
Log(level, "Deflate block header bfinal=",
91+
static_cast<int>(data[header_length] & 0b00000001),
92+
", btype=", static_cast<int>((data[header_length] & 0b00000110) >> 1),
93+
"\n", std::forward<Args>(args)...);
9094
}
9195
}
9296
#else
93-
#define PrintDeflateBlockHeader(...)
97+
template <typename... Args>
98+
inline void PrintDeflateBlockHeader(LogLevel, uint8_t*, uint32_t, int,
99+
Args&&...) {}
94100
#endif

‎statistics.cpp‎

Lines changed: 18 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -4,75 +4,37 @@
44
#include "statistics.h"
55

66
#ifdef ENABLE_STATISTICS
7+
#include <algorithm>
8+
#include <array>
79
#include <sstream>
810
#include <thread>
911

1012
#include "config/config.h"
1113
#include "logging.h"
12-
using namespace config;
13-
14-
// clang-format off
15-
const std::string stat_names[STATS_COUNT] {
16-
"deflate_count",
17-
"deflate_error_count",
18-
"deflate_qat_count",
19-
"deflate_qat_error_count",
20-
"deflate_iaa_count",
21-
"deflate_iaa_error_count",
22-
"deflate_zlib_count",
23-
24-
"inflate_count",
25-
"inflate_error_count",
26-
"inflate_qat_count",
27-
"inflate_qat_error_count",
28-
"inflate_iaa_count",
29-
"inflate_iaa_error_count",
30-
"inflate_zlib_count"
31-
};
32-
// clang-format on
33-
34-
thread_local uint64_t stats[STATS_COUNT];
35-
#endif
3614

37-
bool AreStatsEnabled() {
38-
#ifdef ENABLE_STATISTICS
39-
return true;
40-
#else
41-
return false;
42-
#endif
43-
}
15+
using namespace config;
4416

45-
void ResetStats() {
46-
#ifdef ENABLE_STATISTICS
47-
for (int i = 0; i < STATS_COUNT; i++) {
48-
stats[i] = 0;
49-
}
50-
#endif
51-
}
17+
const std::array<const char*, STATS_COUNT> stat_names{
18+
{"deflate_count", "deflate_error_count", "deflate_qat_count",
19+
"deflate_qat_error_count", "deflate_iaa_count", "deflate_iaa_error_count",
20+
"deflate_zlib_count", "inflate_count", "inflate_error_count",
21+
"inflate_qat_count", "inflate_qat_error_count", "inflate_iaa_count",
22+
"inflate_iaa_error_count", "inflate_zlib_count"}};
5223

53-
uint64_t GetStat(Statistic stat) {
54-
#ifdef ENABLE_STATISTICS
55-
return stats[stat];
56-
#else
57-
(void)stat;
58-
return 0;
59-
#endif
60-
}
24+
thread_local std::array<uint64_t, STATS_COUNT> stats{};
6125

62-
#ifdef ENABLE_STATISTICS
6326
void PrintStats() {
64-
if ((stats[DEFLATE_COUNT] + stats[INFLATE_COUNT]) %
65-
configs[LOG_STATS_SAMPLES] !=
66-
0) {
27+
auto total_operations = stats[static_cast<size_t>(Statistic::DEFLATE_COUNT)] +
28+
stats[static_cast<size_t>(Statistic::INFLATE_COUNT)];
29+
30+
if (total_operations % configs[LOG_STATS_SAMPLES] != 0) {
6731
return;
6832
}
6933

70-
std::stringstream printed_stats;
71-
printed_stats << "Thread: " << std::this_thread::get_id() << std::endl;
72-
for (int i = 0; i < STATS_COUNT; i++) {
73-
printed_stats << stat_names[i] << " = " << stats[i] << std::endl;
74-
}
34+
LogStats("Thread: ", std::this_thread::get_id(), "\n");
7535

76-
LogStats(printed_stats.str().c_str());
36+
for (size_t i = 0; i < stats.size(); ++i) {
37+
LogStats(stat_names[i], " = ", stats[i], "\n");
38+
}
7739
}
7840
#endif

‎statistics.h‎

Lines changed: 45 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -3,45 +3,73 @@
33

44
#pragma once
55

6-
#define VISIBLE_FOR_TESTING __attribute__((visibility("default")))
7-
#include <stdint.h>
6+
#include <array>
7+
#include <cstddef>
8+
#include <cstdint>
9+
#include <ostream>
810

9-
#ifdef ENABLE_STATISTICS
10-
#define INCREMENT_STAT(stat) stats[stat]++
11-
#define INCREMENT_STAT_COND(cond, stat) \
12-
if (cond) stats[stat]++
13-
#else
14-
#define INCREMENT_STAT(stat)
15-
#define INCREMENT_STAT_COND(cond, stat)
16-
#endif
11+
#define VISIBLE_FOR_TESTING __attribute__((visibility("default")))
1712

18-
enum Statistic {
13+
enum class Statistic : size_t {
1914
DEFLATE_COUNT = 0,
2015
DEFLATE_ERROR_COUNT,
2116
DEFLATE_QAT_COUNT,
2217
DEFLATE_QAT_ERROR_COUNT,
2318
DEFLATE_IAA_COUNT,
2419
DEFLATE_IAA_ERROR_COUNT,
2520
DEFLATE_ZLIB_COUNT,
26-
2721
INFLATE_COUNT,
2822
INFLATE_ERROR_COUNT,
2923
INFLATE_QAT_COUNT,
3024
INFLATE_QAT_ERROR_COUNT,
3125
INFLATE_IAA_COUNT,
3226
INFLATE_IAA_ERROR_COUNT,
3327
INFLATE_ZLIB_COUNT,
34-
3528
STATS_COUNT
3629
};
3730

31+
constexpr size_t STATS_COUNT = static_cast<size_t>(Statistic::STATS_COUNT);
32+
33+
inline std::ostream& operator<<(std::ostream& os, const Statistic& stat) {
34+
return os << static_cast<size_t>(stat);
35+
}
36+
3837
#ifdef ENABLE_STATISTICS
39-
extern thread_local uint64_t stats[STATS_COUNT];
38+
#define INCREMENT_STAT(stat) stats[static_cast<size_t>(Statistic::stat)]++
39+
#define INCREMENT_STAT_COND(cond, stat) \
40+
if (cond) stats[static_cast<size_t>(Statistic::stat)]++
41+
#else
42+
#define INCREMENT_STAT(stat)
43+
#define INCREMENT_STAT_COND(cond, stat)
44+
#endif
45+
46+
#ifdef ENABLE_STATISTICS
47+
extern thread_local std::array<uint64_t, STATS_COUNT> stats;
48+
extern const std::array<const char*, STATS_COUNT> stat_names;
49+
#endif
50+
51+
VISIBLE_FOR_TESTING constexpr bool AreStatsEnabled() {
52+
#ifdef ENABLE_STATISTICS
53+
return true;
54+
#else
55+
return false;
56+
#endif
57+
}
58+
59+
VISIBLE_FOR_TESTING inline void ResetStats() {
60+
#ifdef ENABLE_STATISTICS
61+
stats.fill(0);
4062
#endif
63+
}
4164

42-
VISIBLE_FOR_TESTING bool AreStatsEnabled();
43-
VISIBLE_FOR_TESTING void ResetStats();
44-
VISIBLE_FOR_TESTING uint64_t GetStat(Statistic stat);
65+
VISIBLE_FOR_TESTING inline uint64_t GetStat(Statistic stat) {
66+
#ifdef ENABLE_STATISTICS
67+
return stats[static_cast<size_t>(stat)];
68+
#else
69+
(void)stat;
70+
return 0;
71+
#endif
72+
}
4573

4674
#ifdef ENABLE_STATISTICS
4775
void PrintStats();

0 commit comments

Comments
 (0)