From bad9c58e608498e62ec321f59c78d86aa4ef9e8a Mon Sep 17 00:00:00 2001 From: Dmitry Tsarevich Date: Mon, 15 May 2023 16:59:44 +0300 Subject: [PATCH] Fix found by PVS-Studio issues (#490) Small changes, essentially cleanup, with no actual code logic change. * Ensure symbol_buffer aligned same as SYMBOL_INFO * Use constexpr for compile-time constant * Use = default for ctor bodies to allow compiler optimize them * Pass string by reference to prevent copying, other smaller types we can avoid passing by reference as the type is cheaper to copy. --- src/filesink.cpp | 2 +- src/filesinkhelper.ipp | 4 ++-- src/g3log/atomicbool.hpp | 8 ++++---- src/g3log/logmessage.hpp | 5 +++-- src/g3log/shared_queue.hpp | 2 +- src/g3log/sinkhandle.hpp | 2 +- src/g3log/time.hpp | 2 +- src/logmessage.cpp | 2 +- src/stacktrace_windows.cpp | 4 ++-- src/time.cpp | 2 +- 10 files changed, 17 insertions(+), 16 deletions(-) diff --git a/src/filesink.cpp b/src/filesink.cpp index 5058697..a7e0f26 100644 --- a/src/filesink.cpp +++ b/src/filesink.cpp @@ -92,7 +92,7 @@ namespace g3 { ss_change.str(""); std::string old_log = _log_file_with_path; - _log_file_with_path = prospect_log; + _log_file_with_path = std::move(prospect_log); _outptr = std::move(log_stream); ss_change << "\n\tNew log file. The previous log file was at: "; ss_change << old_log << "\n"; diff --git a/src/filesinkhelper.ipp b/src/filesinkhelper.ipp index 0cde5e1..1e42999 100644 --- a/src/filesinkhelper.ipp +++ b/src/filesinkhelper.ipp @@ -51,7 +51,7 @@ namespace g3 { return prefix; } - std::string pathSanityFix(std::string path, std::string file_name) { + std::string pathSanityFix(std::string path, const std::string &file_name) { // Unify the delimeters,. maybe sketchy solution but it seems to work // on at least win7 + ubuntu. All bets are off for older windows std::replace(path.begin(), path.end(), '\\', '/'); @@ -88,7 +88,7 @@ namespace g3 { std::string createLogFileName(const std::string &verified_prefix, const std::string &logger_id) { std::stringstream oss_name; oss_name << verified_prefix << "."; - if( logger_id != "" ) { + if( !logger_id.empty() ) { oss_name << logger_id << "."; } auto now = std::chrono::system_clock::now(); diff --git a/src/g3log/atomicbool.hpp b/src/g3log/atomicbool.hpp index f5ab021..022cd6e 100644 --- a/src/g3log/atomicbool.hpp +++ b/src/g3log/atomicbool.hpp @@ -18,7 +18,7 @@ namespace g3 { std::atomic value_; public: atomicbool(): value_ {false} {} - atomicbool(const bool& value): value_ {value} {} + atomicbool(bool value): value_ {value} {} atomicbool(const std::atomic& value) : value_ {value.load(std::memory_order_acquire)} {} atomicbool(const atomicbool& other): value_ {other.value_.load(std::memory_order_acquire)} {} @@ -32,9 +32,9 @@ namespace g3 { return *this; } - bool operator==(const atomicbool& rhs) const { - return (value_.load(std::memory_order_acquire) == rhs.value_.load(std::memory_order_acquire)); - } + bool operator==(const atomicbool& rhs) const { + return (value_.load(std::memory_order_acquire) == rhs.value_.load(std::memory_order_acquire)); + } bool value() {return value_.load(std::memory_order_acquire);} std::atomic& get() {return value_;} diff --git a/src/g3log/logmessage.hpp b/src/g3log/logmessage.hpp index 83795ee..1258539 100644 --- a/src/g3log/logmessage.hpp +++ b/src/g3log/logmessage.hpp @@ -67,8 +67,8 @@ namespace g3 { std::string threadID() const; - void setExpression(const std::string expression) { - _expression = expression; + void setExpression(std::string expression) { + _expression = std::move(expression); } @@ -149,6 +149,7 @@ namespace g3 { struct FatalMessage : public LogMessage { FatalMessage(const LogMessage& details, g3::SignalType signal_id); FatalMessage(const FatalMessage&); + FatalMessage& operator=(const FatalMessage&) = delete; virtual ~FatalMessage() {} LogMessage copyToLogMessage() const; diff --git a/src/g3log/shared_queue.hpp b/src/g3log/shared_queue.hpp index 55af50a..27da74b 100644 --- a/src/g3log/shared_queue.hpp +++ b/src/g3log/shared_queue.hpp @@ -34,7 +34,7 @@ class shared_queue shared_queue(const shared_queue &other) = delete; public: - shared_queue() {} + shared_queue() = default; void push(T item) { { diff --git a/src/g3log/sinkhandle.hpp b/src/g3log/sinkhandle.hpp index 414d74e..34c53b4 100644 --- a/src/g3log/sinkhandle.hpp +++ b/src/g3log/sinkhandle.hpp @@ -30,7 +30,7 @@ namespace g3 { SinkHandle(std::shared_ptr> sink) : _sink(sink) {} - ~SinkHandle() {} + ~SinkHandle() = default; // Asynchronous call to the real sink. If the real sink is already deleted diff --git a/src/g3log/time.hpp b/src/g3log/time.hpp index d264362..6598e00 100644 --- a/src/g3log/time.hpp +++ b/src/g3log/time.hpp @@ -49,7 +49,7 @@ namespace g3 { /** return time representing POD struct (ref ctime + wchar) that is normally * retrieved with std::localtime. g3::localtime is threadsafe which std::localtime is not. * g3::localtime is probably used together with @ref g3::systemtime_now */ - tm localtime(const std::time_t& time); + tm localtime(std::time_t time); /** format string must conform to std::put_time's demands. * WARNING: At time of writing there is only so-so compiler support for diff --git a/src/logmessage.cpp b/src/logmessage.cpp index e12a884..d69302a 100644 --- a/src/logmessage.cpp +++ b/src/logmessage.cpp @@ -164,7 +164,7 @@ namespace g3 { #endif , _file_path(file) , _line(line) - , _function(function) + , _function(std::move(function)) , _level(level) { } diff --git a/src/stacktrace_windows.cpp b/src/stacktrace_windows.cpp index 60ef6c7..427e4aa 100644 --- a/src/stacktrace_windows.cpp +++ b/src/stacktrace_windows.cpp @@ -120,7 +120,7 @@ namespace { DWORD64 displacement64; DWORD displacement; - char symbol_buffer[sizeof(SYMBOL_INFO) + MAX_SYM_NAME]; + alignas(SYMBOL_INFO) char symbol_buffer[sizeof(SYMBOL_INFO) + MAX_SYM_NAME]; SYMBOL_INFO *symbol = reinterpret_cast(symbol_buffer); symbol->SizeOfStruct = sizeof(SYMBOL_INFO); symbol->MaxNameLen = MAX_SYM_NAME; @@ -216,7 +216,7 @@ namespace stacktrace { }); // Raii sym cleanup - const size_t kmax_frame_dump_size = 64; + constexpr size_t kmax_frame_dump_size = 64; std::vector frame_pointers(kmax_frame_dump_size); // C++11: size set and values are zeroed diff --git a/src/time.cpp b/src/time.cpp index 4c0dc25..bdf91f0 100644 --- a/src/time.cpp +++ b/src/time.cpp @@ -129,7 +129,7 @@ namespace g3 { - tm localtime(const std::time_t& ts) { + tm localtime(std::time_t ts) { struct tm tm_snapshot; #if (defined(WIN32) || defined(_WIN32) || defined(__WIN32__)) localtime_s(&tm_snapshot, &ts); // windsows