Skip to content

Commit d28ccec

Browse files
IronsDuclaude
IronsDu
and
claude
committed
fix: resolve CI warning and format check failures
- Remove unused levelToString function in default_log_sink.cpp - Apply clang-format to all logger-related files Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent 3708d63 commit d28ccec

8 files changed

Lines changed: 60 additions & 128 deletions

File tree

‎include/profiler/log_sink.h‎

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,12 @@ PROFILER_NAMESPACE_BEGIN
1111
/// @enum LogLevel
1212
/// @brief Log severity levels
1313
enum class LogLevel {
14-
Trace, ///< Detailed debug information
15-
Debug, ///< Debug information
16-
Info, ///< General information
17-
Warning, ///< Warning messages
18-
Error, ///< Error messages
19-
Fatal ///< Fatal errors (typically terminates program)
14+
Trace, ///< Detailed debug information
15+
Debug, ///< Debug information
16+
Info, ///< General information
17+
Warning, ///< Warning messages
18+
Error, ///< Error messages
19+
Fatal ///< Fatal errors (typically terminates program)
2020
};
2121

2222
/// @class LogSink
@@ -49,11 +49,7 @@ class LogSink {
4949
/// @param line Source line number
5050
/// @param function Function name (may be nullptr)
5151
/// @param message Formatted log message
52-
virtual void log(LogLevel level,
53-
const char* file,
54-
int line,
55-
const char* function,
56-
const char* message) = 0;
52+
virtual void log(LogLevel level, const char* file, int line, const char* function, const char* message) = 0;
5753

5854
/// @brief Flush any buffered log output
5955
/// @note Default implementation does nothing

‎src/internal/default_log_sink.cpp‎

Lines changed: 22 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,10 @@
22
/// @brief Default log sink implementation using spdlog
33

44
#include "default_log_sink.h"
5-
#include <spdlog/spdlog.h>
6-
#include <spdlog/sinks/stdout_color_sinks.h>
7-
#include <spdlog/pattern_formatter.h>
85
#include <mutex>
6+
#include <spdlog/pattern_formatter.h>
7+
#include <spdlog/sinks/stdout_color_sinks.h>
8+
#include <spdlog/spdlog.h>
99

1010
PROFILER_NAMESPACE_BEGIN
1111

@@ -14,40 +14,20 @@ namespace internal {
1414
// Helper to convert profiler LogLevel to spdlog level
1515
static spdlog::level::level_enum toSpdlogLevel(LogLevel level) {
1616
switch (level) {
17-
case LogLevel::Trace:
18-
return spdlog::level::trace;
19-
case LogLevel::Debug:
20-
return spdlog::level::debug;
21-
case LogLevel::Info:
22-
return spdlog::level::info;
23-
case LogLevel::Warning:
24-
return spdlog::level::warn;
25-
case LogLevel::Error:
26-
return spdlog::level::err;
27-
case LogLevel::Fatal:
28-
return spdlog::level::critical;
29-
default:
30-
return spdlog::level::info;
31-
}
32-
}
33-
34-
// Helper to convert LogLevel to string
35-
static const char* levelToString(LogLevel level) {
36-
switch (level) {
37-
case LogLevel::Trace:
38-
return "TRACE";
39-
case LogLevel::Debug:
40-
return "DEBUG";
41-
case LogLevel::Info:
42-
return "INFO";
43-
case LogLevel::Warning:
44-
return "WARN";
45-
case LogLevel::Error:
46-
return "ERROR";
47-
case LogLevel::Fatal:
48-
return "FATAL";
49-
default:
50-
return "UNKNOWN";
17+
case LogLevel::Trace:
18+
return spdlog::level::trace;
19+
case LogLevel::Debug:
20+
return spdlog::level::debug;
21+
case LogLevel::Info:
22+
return spdlog::level::info;
23+
case LogLevel::Warning:
24+
return spdlog::level::warn;
25+
case LogLevel::Error:
26+
return spdlog::level::err;
27+
case LogLevel::Fatal:
28+
return spdlog::level::critical;
29+
default:
30+
return spdlog::level::info;
5131
}
5232
}
5333

@@ -61,7 +41,7 @@ class DefaultLogSink::Impl {
6141
// Format: [timestamp] [level] [source_location] message
6242
logger_ = std::make_shared<spdlog::logger>("profiler", stderr_sink);
6343
logger_->set_pattern("[%Y-%m-%d %H:%M:%S.%e] [%^%-7l%$] [%s:%#] %v");
64-
logger_->set_level(spdlog::level::trace); // Let LogManager handle filtering
44+
logger_->set_level(spdlog::level::trace); // Let LogManager handle filtering
6545

6646
// Register as default logger (optional, for spdlog internal use)
6747
spdlog::register_logger(logger_);
@@ -71,11 +51,7 @@ class DefaultLogSink::Impl {
7151
spdlog::drop("profiler");
7252
}
7353

74-
void log(LogLevel level,
75-
const char* file,
76-
int line,
77-
const char* function,
78-
const char* message) {
54+
void log(LogLevel level, const char* file, int line, const char* function, const char* message) {
7955
// Extract just the filename from the full path
8056
const char* filename = file;
8157
if (const char* last_slash = strrchr(file, '/')) {
@@ -106,17 +82,11 @@ class DefaultLogSink::Impl {
10682
std::shared_ptr<spdlog::logger> logger_;
10783
};
10884

109-
DefaultLogSink::DefaultLogSink()
110-
: impl_(std::make_unique<Impl>()) {
111-
}
85+
DefaultLogSink::DefaultLogSink() : impl_(std::make_unique<Impl>()) {}
11286

11387
DefaultLogSink::~DefaultLogSink() = default;
11488

115-
void DefaultLogSink::log(LogLevel level,
116-
const char* file,
117-
int line,
118-
const char* function,
119-
const char* message) {
89+
void DefaultLogSink::log(LogLevel level, const char* file, int line, const char* function, const char* message) {
12090
std::lock_guard<std::mutex> lock(mutex_);
12191
impl_->log(level, file, line, function, message);
12292
}
@@ -131,6 +101,6 @@ void DefaultLogSink::setLogLevel(LogLevel level) {
131101
impl_->setLogLevel(level);
132102
}
133103

134-
} // namespace internal
104+
} // namespace internal
135105

136106
PROFILER_NAMESPACE_END

‎src/internal/default_log_sink.h‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,7 @@ class DefaultLogSink : public LogSink {
1818
DefaultLogSink();
1919
~DefaultLogSink() override;
2020

21-
void log(LogLevel level,
22-
const char* file,
23-
int line,
24-
const char* function,
25-
const char* message) override;
21+
void log(LogLevel level, const char* file, int line, const char* function, const char* message) override;
2622

2723
void flush() override;
2824

@@ -35,6 +31,6 @@ class DefaultLogSink : public LogSink {
3531
std::unique_ptr<Impl> impl_;
3632
};
3733

38-
} // namespace internal
34+
} // namespace internal
3935

4036
PROFILER_NAMESPACE_END

‎src/internal/log_macros.h‎

Lines changed: 14 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -11,40 +11,32 @@ PROFILER_NAMESPACE_BEGIN
1111
namespace internal {
1212

1313
// Helper template to format message using fmt
14-
template <typename... Args>
15-
std::string formatMessage(fmt::format_string<Args...> fmt, Args&&... args) {
14+
template <typename... Args> std::string formatMessage(fmt::format_string<Args...> fmt, Args&&... args) {
1615
return fmt::format(fmt, std::forward<Args>(args)...);
1716
}
1817

19-
} // namespace internal
18+
} // namespace internal
2019

2120
PROFILER_NAMESPACE_END
2221

2322
// Internal logging macro using fmt-style formatting
24-
#define PROFILER_LOG_IMPL(level, fmt_str, ...) \
25-
do { \
26-
if (::profiler::internal::LogManager::instance().shouldLog(level)) { \
27-
::profiler::internal::logMessage(level, __FILE__, __LINE__, \
28-
__FUNCTION__, \
29-
::profiler::internal::formatMessage(fmt_str, ##__VA_ARGS__)); \
30-
} \
23+
#define PROFILER_LOG_IMPL(level, fmt_str, ...) \
24+
do { \
25+
if (::profiler::internal::LogManager::instance().shouldLog(level)) { \
26+
::profiler::internal::logMessage(level, __FILE__, __LINE__, __FUNCTION__, \
27+
::profiler::internal::formatMessage(fmt_str, ##__VA_ARGS__)); \
28+
} \
3129
} while (0)
3230

3331
// User-facing logging macros (fmt-style: PROFILER_INFO("Message: {}", value))
34-
#define PROFILER_TRACE(fmt_str, ...) \
35-
PROFILER_LOG_IMPL(::profiler::LogLevel::Trace, fmt_str, ##__VA_ARGS__)
32+
#define PROFILER_TRACE(fmt_str, ...) PROFILER_LOG_IMPL(::profiler::LogLevel::Trace, fmt_str, ##__VA_ARGS__)
3633

37-
#define PROFILER_DEBUG(fmt_str, ...) \
38-
PROFILER_LOG_IMPL(::profiler::LogLevel::Debug, fmt_str, ##__VA_ARGS__)
34+
#define PROFILER_DEBUG(fmt_str, ...) PROFILER_LOG_IMPL(::profiler::LogLevel::Debug, fmt_str, ##__VA_ARGS__)
3935

40-
#define PROFILER_INFO(fmt_str, ...) \
41-
PROFILER_LOG_IMPL(::profiler::LogLevel::Info, fmt_str, ##__VA_ARGS__)
36+
#define PROFILER_INFO(fmt_str, ...) PROFILER_LOG_IMPL(::profiler::LogLevel::Info, fmt_str, ##__VA_ARGS__)
4237

43-
#define PROFILER_WARNING(fmt_str, ...) \
44-
PROFILER_LOG_IMPL(::profiler::LogLevel::Warning, fmt_str, ##__VA_ARGS__)
38+
#define PROFILER_WARNING(fmt_str, ...) PROFILER_LOG_IMPL(::profiler::LogLevel::Warning, fmt_str, ##__VA_ARGS__)
4539

46-
#define PROFILER_ERROR(fmt_str, ...) \
47-
PROFILER_LOG_IMPL(::profiler::LogLevel::Error, fmt_str, ##__VA_ARGS__)
40+
#define PROFILER_ERROR(fmt_str, ...) PROFILER_LOG_IMPL(::profiler::LogLevel::Error, fmt_str, ##__VA_ARGS__)
4841

49-
#define PROFILER_FATAL(fmt_str, ...) \
50-
PROFILER_LOG_IMPL(::profiler::LogLevel::Fatal, fmt_str, ##__VA_ARGS__)
42+
#define PROFILER_FATAL(fmt_str, ...) PROFILER_LOG_IMPL(::profiler::LogLevel::Fatal, fmt_str, ##__VA_ARGS__)

‎src/internal/log_manager.cpp‎

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,7 @@ LogManager& LogManager::instance() {
1414
return instance;
1515
}
1616

17-
LogManager::LogManager()
18-
: sink_(std::make_shared<DefaultLogSink>()) {
19-
}
17+
LogManager::LogManager() : sink_(std::make_shared<DefaultLogSink>()) {}
2018

2119
void LogManager::setSink(std::shared_ptr<LogSink> sink) {
2220
std::lock_guard<std::mutex> lock(mutex_);
@@ -45,15 +43,11 @@ bool LogManager::shouldLog(LogLevel level) const {
4543
return static_cast<int>(level) >= static_cast<int>(level_.load(std::memory_order_relaxed));
4644
}
4745

48-
void logMessage(LogLevel level,
49-
const char* file,
50-
int line,
51-
const char* function,
52-
std::string&& message) {
46+
void logMessage(LogLevel level, const char* file, int line, const char* function, std::string&& message) {
5347
LogManager::instance().sink()->log(level, file, line, function, message.c_str());
5448
}
5549

56-
} // namespace internal
50+
} // namespace internal
5751

5852
// Implementation of public API functions (in profiler namespace)
5953
void setSink(std::shared_ptr<LogSink> sink) {

‎src/internal/log_manager.h‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -46,12 +46,8 @@ class LogManager {
4646
};
4747

4848
/// @brief Internal function to write a log message (fmt-style formatting)
49-
void logMessage(LogLevel level,
50-
const char* file,
51-
int line,
52-
const char* function,
53-
std::string&& message);
49+
void logMessage(LogLevel level, const char* file, int line, const char* function, std::string&& message);
5450

55-
} // namespace internal
51+
} // namespace internal
5652

5753
PROFILER_NAMESPACE_END

‎src/profiler_manager.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,8 @@ void ProfilerManager::setStackCaptureSignal(int signal) {
117117

118118
// If handler is already installed, restore old one first
119119
if (old_action_saved_) {
120-
PROFILER_WARNING("Signal handler already installed for signal {}. Restoring before changing.", stack_capture_signal_);
120+
PROFILER_WARNING("Signal handler already installed for signal {}. Restoring before changing.",
121+
stack_capture_signal_);
121122
// Note: This won't work correctly if multiple instances exist,
122123
// but it's a best-effort attempt
123124
ProfilerManager::getInstance().restoreSignalHandler();
@@ -147,8 +148,7 @@ void ProfilerManager::installSignalHandler() {
147148
old_action_saved_ = true;
148149
PROFILER_INFO("Registered signal handler for signal {}", stack_capture_signal_);
149150
} else {
150-
PROFILER_ERROR("Failed to register signal handler for signal {}: {}",
151-
stack_capture_signal_, strerror(errno));
151+
PROFILER_ERROR("Failed to register signal handler for signal {}: {}", stack_capture_signal_, strerror(errno));
152152
}
153153
}
154154

‎tests/test_logger.cpp‎

Lines changed: 7 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
/// @file test_logger.cpp
22
/// @brief Tests for the custom logger sink functionality
33

4+
#include <atomic>
45
#include <gtest/gtest.h>
6+
#include <mutex>
57
#include <profiler/log_sink.h>
68
#include <profiler/logger.h>
7-
#include <atomic>
8-
#include <mutex>
99
#include <vector>
1010

1111
PROFILER_NAMESPACE_BEGIN
@@ -21,19 +21,9 @@ class MockLogSink : public LogSink {
2121
std::string message;
2222
};
2323

24-
void log(LogLevel level,
25-
const char* file,
26-
int line,
27-
const char* function,
28-
const char* message) override {
24+
void log(LogLevel level, const char* file, int line, const char* function, const char* message) override {
2925
std::lock_guard<std::mutex> lock(mutex_);
30-
entries_.push_back(LogEntry{
31-
level,
32-
file ? file : "",
33-
line,
34-
function ? function : "",
35-
message ? message : ""
36-
});
26+
entries_.push_back(LogEntry{level, file ? file : "", line, function ? function : "", message ? message : ""});
3727
log_count_++;
3828
}
3929

@@ -116,7 +106,7 @@ TEST_F(LoggerTest, LogLevelFiltering) {
116106

117107
// Log messages at different levels directly to sink
118108
// (The filtering happens in LogManager, but we can verify the level is set)
119-
EXPECT_TRUE(true); // Level is set
109+
EXPECT_TRUE(true); // Level is set
120110

121111
// Reset to trace for other tests
122112
setLogLevel(LogLevel::Trace);
@@ -177,10 +167,8 @@ TEST_F(LoggerTest, ThreadSafety) {
177167
for (int t = 0; t < num_threads; ++t) {
178168
threads.emplace_back([this, t, messages_per_thread]() {
179169
for (int i = 0; i < messages_per_thread; ++i) {
180-
mock_sink_->log(LogLevel::Info, "thread_test.cpp",
181-
t * messages_per_thread + i,
182-
"threadFunc",
183-
("Thread " + std::to_string(t) + " message " + std::to_string(i)).c_str());
170+
mock_sink_->log(LogLevel::Info, "thread_test.cpp", t * messages_per_thread + i, "threadFunc",
171+
("Thread " + std::to_string(t) + " message " + std::to_string(i)).c_str());
184172
}
185173
});
186174
}

0 commit comments

Comments
 (0)