Skip to content

Commit

Permalink
Fix found by PVS-Studio issues (#490)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
dimhotepus authored May 15, 2023
1 parent cc0fb7c commit bad9c58
Show file tree
Hide file tree
Showing 10 changed files with 17 additions and 16 deletions.
2 changes: 1 addition & 1 deletion src/filesink.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down
4 changes: 2 additions & 2 deletions src/filesinkhelper.ipp
Original file line number Diff line number Diff line change
Expand Up @@ -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(), '\\', '/');
Expand Down Expand Up @@ -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();
Expand Down
8 changes: 4 additions & 4 deletions src/g3log/atomicbool.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ namespace g3 {
std::atomic<bool> value_;
public:
atomicbool(): value_ {false} {}
atomicbool(const bool& value): value_ {value} {}
atomicbool(bool value): value_ {value} {}
atomicbool(const std::atomic<bool>& value) : value_ {value.load(std::memory_order_acquire)} {}
atomicbool(const atomicbool& other): value_ {other.value_.load(std::memory_order_acquire)} {}

Expand All @@ -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<bool>& get() {return value_;}
Expand Down
5 changes: 3 additions & 2 deletions src/g3log/logmessage.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}


Expand Down Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion src/g3log/shared_queue.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ class shared_queue
shared_queue(const shared_queue &other) = delete;

public:
shared_queue() {}
shared_queue() = default;

void push(T item) {
{
Expand Down
2 changes: 1 addition & 1 deletion src/g3log/sinkhandle.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ namespace g3 {
SinkHandle(std::shared_ptr<internal::Sink<T>> sink)
: _sink(sink) {}

~SinkHandle() {}
~SinkHandle() = default;


// Asynchronous call to the real sink. If the real sink is already deleted
Expand Down
2 changes: 1 addition & 1 deletion src/g3log/time.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/logmessage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ namespace g3 {
#endif
, _file_path(file)
, _line(line)
, _function(function)
, _function(std::move(function))
, _level(level) {
}

Expand Down
4 changes: 2 additions & 2 deletions src/stacktrace_windows.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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_INFO *>(symbol_buffer);
symbol->SizeOfStruct = sizeof(SYMBOL_INFO);
symbol->MaxNameLen = MAX_SYM_NAME;
Expand Down Expand Up @@ -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<uint64_t> frame_pointers(kmax_frame_dump_size);
// C++11: size set and values are zeroed

Expand Down
2 changes: 1 addition & 1 deletion src/time.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit bad9c58

Please sign in to comment.