Skip to content

Commit

Permalink
Update static-analysis + Fix various sonar issues (#357)
Browse files Browse the repository at this point in the history
* Refactor complex telnet function + Add sentry comments

* limit nest to 3

* reduce complexity

* fix format issues

* replace static call

* format

* clang-tidy fixes

* more clang-tidy fixes

* add comment

* move to anonymous

* move to anonymous

* more fixes

* last fixes
  • Loading branch information
egecetin authored Dec 8, 2024
1 parent 0e7a410 commit 35c4040
Show file tree
Hide file tree
Showing 11 changed files with 72 additions and 70 deletions.
6 changes: 2 additions & 4 deletions include/metrics/ProcessMetrics.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,8 @@ class ProcessMetrics {
size_t _oldReadBytes{0}; ///< Variable to store the old read bytes
size_t _oldWriteBytes{0}; ///< Variable to store the old write bytes

clock_t _oldCpuTime{0}; ///< Variable to store the old CPU time
struct tms _oldCpu {
0, 0, 0, 0
}; ///< Structure to store the old CPU times
clock_t _oldCpuTime{0}; ///< Variable to store the old CPU time
struct tms _oldCpu{0, 0, 0, 0}; ///< Structure to store the old CPU times

/**
* Counts the number of entries in a directory.
Expand Down
8 changes: 7 additions & 1 deletion include/telnet/TelnetServer.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ class TelnetServer : public std::enable_shared_from_this<TelnetServer> {

const VEC_SP_TelnetSession &sessions() const { return m_sessions; }

bool interactivePrompt() const { return m_promptString.length() > 0; }
bool interactivePrompt() const { return !m_promptString.empty(); }
void promptString(const std::string_view &prompt) { m_promptString = prompt; }
const std::string &promptString() const { return m_promptString; }

Expand Down Expand Up @@ -207,6 +207,12 @@ class TelnetServer : public std::enable_shared_from_this<TelnetServer> {
std::shared_ptr<std::atomic_flag> m_checkFlag; /**< Runtime check flag */
};

/**
* Print available commands to the session
* @param[in] session Handle to session
*/
void TelnetPrintAvailableCommands(const SP_TelnetSession &session);

/**
* Telnet session connection start callback
* @param[in] session Handle to session
Expand Down
5 changes: 1 addition & 4 deletions include/utils/BaseServerStats.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,7 @@
#include <prometheus/registry.h>

#define QUANTILE_DEFAULTS \
prometheus::Summary::Quantiles \
{ \
{0.5, 0.1}, {0.9, 0.1}, { 0.99, 0.1 } \
}
prometheus::Summary::Quantiles { {0.5, 0.1}, {0.9, 0.1}, {0.99, 0.1} }

/**
* @class BaseServerStats
Expand Down
2 changes: 1 addition & 1 deletion include/utils/Tracer.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ class Tracer {
* Check if the crashpad_handler process is running
* @return true if the crashpad_handler is running, false otherwise
*/
[[nodiscard]] bool isRunning() const;
[[nodiscard]] static bool isRunning();

/**
* Check and restart the crashpad_handler process if it is not running
Expand Down
17 changes: 12 additions & 5 deletions src/logging/Sentry.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,10 @@
// MAC address length for character string
constexpr int MAC_LEN = 18;

namespace spdlog::sinks
namespace
{
/// Set version related information to the Sentry context
inline void setVersionContext()
void setVersionContext()
{
std::string versionBuffer;
const sentry_value_t versionContext = sentry_value_new_object();
Expand All @@ -32,7 +32,7 @@ namespace spdlog::sinks
}

/// Set host related information to the Sentry context
inline void setHostContext()
void setHostContext()
{
std::array<char, BUFSIZ> hostBuffer{};
gethostname(hostBuffer.data(), BUFSIZ);
Expand All @@ -56,7 +56,7 @@ namespace spdlog::sinks
}

/// Set network related information to the Sentry context
inline void setNetworkContext()
void setNetworkContext()
{
ifaddrs *ifaddr = nullptr;
if (getifaddrs(&ifaddr) < 0)
Expand Down Expand Up @@ -111,7 +111,10 @@ namespace spdlog::sinks

sentry_set_context("Network", networkContext);
}
} // namespace

namespace spdlog::sinks
{
template <typename Mutex> sentry_api_sink<Mutex>::sentry_api_sink(const std::string &sentryAddress)
{
if (sentryAddress.empty())
Expand Down Expand Up @@ -173,12 +176,16 @@ namespace spdlog::sinks
case spdlog::level::info:
case spdlog::level::off:
// For lower levels, do nothing for now. But you can easily handle them here.
break;
default:
break;
}
}

template <typename Mutex> void sentry_api_sink<Mutex>::flush_() {}
template <typename Mutex> void sentry_api_sink<Mutex>::flush_()
{
// Sentry library handles all operations, so no need to flush
}

template class sentry_api_sink<std::mutex>;
template class sentry_api_sink<details::null_mutex>;
Expand Down
7 changes: 4 additions & 3 deletions src/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,13 @@

// SIGALRM interval in seconds
constexpr uintmax_t alarmInterval = 30;
// Interruption flag
// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables)
volatile sig_atomic_t interruptFlag = 0;

namespace
{
// Interruption flag
// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables)
volatile sig_atomic_t interruptFlag = 0;

// Default SIGALRM function
void alarmFunc(int /*unused*/)
{
Expand Down
5 changes: 1 addition & 4 deletions src/metrics/Performance.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,7 @@
#include <prometheus/summary.h>

#define QUANTILE_DEFAULTS \
prometheus::Summary::Quantiles \
{ \
{0.5, 0.1}, {0.9, 0.1}, { 0.99, 0.1 } \
}
prometheus::Summary::Quantiles { {0.5, 0.1}, {0.9, 0.1}, {0.99, 0.1} }

PerformanceTracker::PerformanceTracker(const std::shared_ptr<prometheus::Registry> &reg, const std::string &name,
uint64_t metricID)
Expand Down
6 changes: 3 additions & 3 deletions src/metrics/ProcessMetrics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,14 @@ size_t ProcessMetrics::countDirectoryEntries(const std::string &path)

long int ProcessMetrics::getMemoryUsage()
{
struct rusage r_usage {};
struct rusage r_usage{};
getrusage(RUSAGE_SELF, &r_usage);
return r_usage.ru_maxrss; // NOLINT(cppcoreguidelines-pro-type-union-access)
}

long int ProcessMetrics::getPageFaults()
{
struct rusage r_usage {};
struct rusage r_usage{};
getrusage(RUSAGE_SELF, &r_usage);
return r_usage.ru_majflt; // NOLINT(cppcoreguidelines-pro-type-union-access)
}
Expand All @@ -51,7 +51,7 @@ double ProcessMetrics::getCpuUsage()
return 0.0;
}

struct tms nowCpu {};
struct tms nowCpu{};
auto nowCpuTime = times(&nowCpu);

const double usage = 100.0 * static_cast<double>(nowCpu.tms_utime - _oldCpu.tms_utime) /
Expand Down
65 changes: 28 additions & 37 deletions src/telnet/TelnetServer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

#include <spdlog/spdlog.h>

#include <algorithm>
#include <array>
#include <ctime>
#include <iomanip>
Expand Down Expand Up @@ -129,7 +130,7 @@ void TelnetSession::sendPromptAndBuffer()
}

// Resend the buffer
if (m_buffer.length() > 0)
if (!m_buffer.empty())
{
sendBytes = send(m_socket, m_buffer.c_str(), m_buffer.length(), 0);
if (sendBytes > 0)
Expand Down Expand Up @@ -160,7 +161,7 @@ void TelnetSession::eraseLine()
void TelnetSession::sendLine(std::string data)
{
// If is something is on the prompt, wipe it off
if (m_telnetServer->interactivePrompt() || m_buffer.length() > 0)
if (m_telnetServer->interactivePrompt() || !m_buffer.empty())
{
eraseLine();
}
Expand Down Expand Up @@ -334,25 +335,26 @@ bool TelnetSession::processTab(std::string &buffer)
do
{
found = buffer.find_first_of('\t');
if (found != std::string::npos)
if (found == std::string::npos)
{
// Remove single tab
if (buffer.length() > 0)
{
buffer.erase(found, 1);
}
foundTabs = true;
continue;
}

// Remove single tab
if (!buffer.empty())
{
buffer.erase(found, 1);
}
foundTabs = true;

// Process
if (m_telnetServer->tabCallback())
// Process
if (m_telnetServer->tabCallback())
{
const std::string retCommand = m_telnetServer->tabCallback()(shared_from_this(), buffer.substr(0, found));
if (!retCommand.empty())
{
const std::string retCommand =
m_telnetServer->tabCallback()(shared_from_this(), buffer.substr(0, found));
if (!retCommand.empty())
{
buffer.erase(0, found);
buffer.insert(0, retCommand);
}
buffer.erase(0, found);
buffer.insert(0, retCommand);
}
}
} while (found != std::string::npos);
Expand Down Expand Up @@ -473,14 +475,8 @@ void TelnetSession::update()

// we've got to be careful here. Telnet client might send null characters for New Lines mid-data block. We need
// to swap these out. recv is not null terminated, so its cool
for (size_t i = 0; i < static_cast<size_t>(readBytes); i++)
{
if (recvbuf[i] == ASCII_NULL) // NOLINT(cppcoreguidelines-pro-bounds-constant-array-index)
{
// New Line constant
recvbuf[i] = ASCII_LF; // NOLINT(cppcoreguidelines-pro-bounds-constant-array-index)
}
}
std::replace_if(
recvbuf.begin(), recvbuf.begin() + readBytes, [](char chr) { return chr == ASCII_NULL; }, ASCII_LF);

// Add it to the received buffer
m_buffer.append(recvbuf.data(), static_cast<unsigned int>(readBytes));
Expand Down Expand Up @@ -513,21 +509,16 @@ void TelnetSession::update()
auto lines = getCompleteLines(m_buffer);
for (const auto &line : lines)
{
if (m_telnetServer->newLineCallBack())
if (!m_telnetServer->newLineCallBack())
{
if (m_telnetServer->newLineCallBack()(shared_from_this(), line))
{
++stats.successCmdCtr;
}
else
{
++stats.failCmdCtr;
}
addToHistory(line);
break;
}

m_telnetServer->newLineCallBack()(shared_from_this(), line) ? ++stats.successCmdCtr : ++stats.failCmdCtr;
addToHistory(line);
}

if (m_telnetServer->interactivePrompt() && requirePromptReprint)
if (requirePromptReprint && m_telnetServer->interactivePrompt())
{
eraseLine();
sendPromptAndBuffer();
Expand Down
15 changes: 9 additions & 6 deletions src/utils/ConfigParser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,16 @@

#include <fstream>

template <typename T> std::string stringifyRapidjson(const T &obj)
namespace
{
rapidjson::StringBuffer sbuffer;
rapidjson::Writer<rapidjson::StringBuffer> writer(sbuffer);
obj.Accept(writer);
return sbuffer.GetString();
}
template <typename T> std::string stringifyRapidjson(const T &obj)
{
rapidjson::StringBuffer sbuffer;
rapidjson::Writer<rapidjson::StringBuffer> writer(sbuffer);
obj.Accept(writer);
return sbuffer.GetString();
}
} // namespace

void ConfigParser::readJson()
{
Expand Down
6 changes: 4 additions & 2 deletions src/utils/Tracer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,8 @@ void Tracer::threadFunc() noexcept
{
if (!isRunning())
{
// NOTICE: Worked before but currently restarting the handler raises an error
// due to changes in the crashpad library
restart();
spdlog::warn("Crashpad handler restarted");
}
Expand Down Expand Up @@ -99,12 +101,12 @@ std::string Tracer::getSelfExecutableDir()
return (lastDelimPos == std::string::npos) ? "" : path.substr(0, lastDelimPos);
}

bool Tracer::isRunning() const
bool Tracer::isRunning()
{
int sockId{-1};
pid_t processId{-1};

if (!_clientHandler->GetHandlerSocket(&sockId, &processId))
if (!crashpad::CrashpadClient::GetHandlerSocket(&sockId, &processId))
{
return false;
}
Expand Down

0 comments on commit 35c4040

Please sign in to comment.