Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Address some issues found by sonarqube #165

Merged
merged 4 commits into from
Dec 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/clock.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ void clock::adjust_clock(const duration& adjustment, const double& rtf) {
// Load the current state
const int c = active.load(std::memory_order_relaxed);
const auto& current = data[c];
const int n = int((c + 1) % data.size());
const auto n = int((c + 1) % data.size());
auto& next = data[n];

// Perform the update
Expand Down
15 changes: 8 additions & 7 deletions src/clock.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,23 +23,24 @@
#ifndef NUCLEAR_CLOCK_HPP
#define NUCLEAR_CLOCK_HPP

// Default to using the system clock but allow it to be overridden by the user
#ifndef NUCLEAR_CLOCK_TYPE
#define NUCLEAR_CLOCK_TYPE std::chrono::system_clock
#endif // NUCLEAR_CLOCK_TYPE

#include <array>
#include <atomic>
#include <chrono>
#include <mutex>

namespace NUClear {

// Default to using the system clock but allow it to be overridden by the user
#ifdef NUCLEAR_CLOCK_TYPE
using base_clock = NUCLEAR_CLOCK_TYPE;
#else
using base_clock = std::chrono::steady_clock;
#endif // NUCLEAR_CLOCK_TYPE

/**
* A clock class that extends a base clock type and allows for clock adjustment and setting.
*/
struct clock : NUCLEAR_CLOCK_TYPE {
using base_clock = NUCLEAR_CLOCK_TYPE;
struct clock : base_clock {

/**
* Get the current time of the clock.
Expand Down
4 changes: 2 additions & 2 deletions src/dsl/operation/TypeBind.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,10 @@ namespace dsl {
static void bind(const std::shared_ptr<threading::Reaction>& reaction) {

// Set this reaction as no stats emitting
reaction->emit_stats &= EmitStats<DataType>::value;
reaction->emit_stats = reaction->emit_stats && EmitStats<DataType>::value;

// Our unbinder to remove this reaction
reaction->unbinders.push_back([](const threading::Reaction& r) {
reaction->unbinders.emplace_back([](const threading::Reaction& r) {
auto& vec = store::TypeCallbackStore<DataType>::get();

auto it = std::find_if(
Expand Down
2 changes: 1 addition & 1 deletion src/dsl/word/Always.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ namespace dsl {
static void bind(const std::shared_ptr<threading::Reaction>& reaction) {

// Create an unbinder for the always reaction
reaction->unbinders.push_back([](threading::Reaction& r) {
reaction->unbinders.emplace_back([](threading::Reaction& r) {
r.enabled = false;
// TODO(Alex/Trent) Clean up thread pool too
});
Expand Down
2 changes: 1 addition & 1 deletion src/dsl/word/Every.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ namespace dsl {
template <typename DSL>
static void bind(const std::shared_ptr<threading::Reaction>& reaction, NUClear::clock::duration jump) {

reaction->unbinders.push_back([](const threading::Reaction& r) {
reaction->unbinders.emplace_back([](const threading::Reaction& r) {
r.reactor.emit<emit::Inline>(std::make_unique<operation::Unbind<operation::ChronoTask>>(r.id));
});

Expand Down
2 changes: 1 addition & 1 deletion src/dsl/word/IO.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ namespace dsl {
template <typename DSL>
static void bind(const std::shared_ptr<threading::Reaction>& reaction, fd_t fd, event_t watch_set) {

reaction->unbinders.push_back([](const threading::Reaction& r) {
reaction->unbinders.emplace_back([](const threading::Reaction& r) {
r.reactor.emit<emit::Inline>(std::make_unique<operation::Unbind<IO>>(r.id));
});

Expand Down
2 changes: 1 addition & 1 deletion src/dsl/word/Idle.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ namespace dsl {
const std::shared_ptr<const util::ThreadPoolDescriptor>& pool_descriptor) {

// Our unbinder to remove this reaction
reaction->unbinders.push_back([pool_descriptor](const threading::Reaction& r) { //
reaction->unbinders.emplace_back([pool_descriptor](const threading::Reaction& r) { //
r.reactor.powerplant.remove_idle_task(r.id, pool_descriptor);
});

Expand Down
2 changes: 1 addition & 1 deletion src/dsl/word/Network.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ namespace dsl {
auto task = std::make_unique<NetworkListen>();

task->hash = util::serialise::Serialise<T>::hash();
reaction->unbinders.push_back([](const threading::Reaction& r) {
reaction->unbinders.emplace_back([](const threading::Reaction& r) {
r.reactor.emit<emit::Inline>(std::make_unique<operation::Unbind<NetworkListen>>(r.id));
});

Expand Down
2 changes: 1 addition & 1 deletion src/dsl/word/TCP.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ namespace dsl {
// Generate a reaction for the IO system that closes on death
const fd_t cfd = fd.release();

reaction->unbinders.push_back([cfd](const threading::Reaction&) {
reaction->unbinders.emplace_back([cfd](const threading::Reaction&) {
::shutdown(cfd, SHUT_RDWR);
::close(cfd);
});
Expand Down
4 changes: 2 additions & 2 deletions src/dsl/word/UDP.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,7 @@ namespace dsl {

// Generate a reaction for the IO system that closes on death
const fd_t cfd = fd.release();
reaction->unbinders.push_back([cfd](const threading::Reaction&) { ::close(cfd); });
reaction->unbinders.emplace_back([cfd](const threading::Reaction&) { ::close(cfd); });
IO::bind<DSL>(reaction, cfd, IO::READ | IO::CLOSE);

// Return our handles and our bound port
Expand Down Expand Up @@ -520,7 +520,7 @@ namespace dsl {

const auto& a = result.local;
const bool multicast =
(a.sock.sa_family == AF_INET && (ntohl(a.ipv4.sin_addr.s_addr) & 0xF0000000) == 0xE0000000)
(a.sock.sa_family == AF_INET && (ntohl(a.ipv4.sin_addr.s_addr) & 0xF0000000U) == 0xE0000000U)
|| (a.sock.sa_family == AF_INET6 && a.ipv6.sin6_addr.s6_addr[0] == 0xFF);

// Only return multicast packets
Expand Down
4 changes: 2 additions & 2 deletions src/dsl/word/Watchdog.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ namespace dsl {
WatchdogDataStore<WatchdogGroup, RuntimeType>::init(data);

// Create our unbinder
reaction->unbinders.push_back([data](const threading::Reaction& r) {
reaction->unbinders.emplace_back([data](const threading::Reaction& r) {
// Remove the active service time from the data store
WatchdogDataStore<WatchdogGroup, RuntimeType>::unbind(data);
r.reactor.emit<emit::Inline>(std::make_unique<operation::Unbind<operation::ChronoTask>>(r.id));
Expand All @@ -237,7 +237,7 @@ namespace dsl {
WatchdogDataStore<WatchdogGroup>::init();

// Create our unbinder
reaction->unbinders.push_back([](const threading::Reaction& r) {
reaction->unbinders.emplace_back([](const threading::Reaction& r) {
// Remove the active service time from the data store
WatchdogDataStore<WatchdogGroup>::unbind();
r.reactor.emit<emit::Inline>(std::make_unique<operation::Unbind<operation::ChronoTask>>(r.id));
Expand Down
2 changes: 1 addition & 1 deletion src/extension/ChronoController.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ namespace extension {

if (clock::rtf() == 0.0) {
// If we are paused then just wait until we are unpaused
wait.wait(lock, [&] {
wait.wait(lock, [this, &start] {
return !running.load(std::memory_order_acquire) || clock::rtf() != 0.0
|| NUClear::clock::now() != start;
});
Expand Down
10 changes: 5 additions & 5 deletions src/extension/NetworkController.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -107,11 +107,11 @@ namespace extension {
const std::lock_guard<std::mutex> lock(reaction_mutex);

// Find and delete this reaction
for (auto it = reactions.begin(); it != reactions.end(); ++it) {
if (it->second->id == unbind.id) {
reactions.erase(it);
break;
}
auto it = std::find_if(reactions.begin(), reactions.end(), [&](const auto& r) {
return r.second->id == unbind.id;
});
if (it != reactions.end()) {
reactions.erase(it);
}
});

Expand Down
4 changes: 2 additions & 2 deletions src/extension/network/NUClearNetwork.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ namespace extension {
// Work out what type of announce we are doing as it will influence how we make the socket
const bool multicast =
(announce_target.sock.sa_family == AF_INET
&& (ntohl(announce_target.ipv4.sin_addr.s_addr) & 0xF0000000) == 0xE0000000)
&& (ntohl(announce_target.ipv4.sin_addr.s_addr) & 0xF0000000U) == 0xE0000000)
|| (announce_target.sock.sa_family == AF_INET6 && announce_target.ipv6.sin6_addr.s6_addr[0] == 0xFF);

// Make our socket
Expand Down Expand Up @@ -920,7 +920,7 @@ namespace extension {
? 0xFF
: 0xFF >> (8 - (packet.packet_count % 8));

all_acked &= static_cast<int>((s->acked[i] & expected) == expected);
all_acked = all_acked && ((s->acked[i] & expected) == expected);
}

// The remote has received this entire packet we can erase our sender
Expand Down
2 changes: 1 addition & 1 deletion src/threading/Reaction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ namespace threading {

void Reaction::unbind() {
// Unbind
for (auto& u : unbinders) {
for (const auto& u : unbinders) {
u(*this);
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/util/Logger.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ namespace util {
auto log_levels = get_current_log_levels(reactor);
if (level >= log_levels.display_log_level || level >= log_levels.min_log_level) {
// Collapse the arguments into a string and perform the log action
do_log(reactor, level, string_join(" ", std::forward<const Arguments&>(args)...));
do_log(reactor, level, string_join(" ", args...));
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/util/get_hostname.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,8 @@ std::string NUClear::util::get_hostname() {
#include <sys/utsname.h>

std::string NUClear::util::get_hostname() {
utsname u{};
uname(&u);
::utsname u{};
::uname(&u);
return u.nodename;
}

Expand Down
8 changes: 2 additions & 6 deletions src/util/serialise/xxhash.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -100,16 +100,14 @@ namespace util {

} // namespace

uint32_t xxhash32(const void* input_v, const size_t& length, const uint32_t& seed) {
uint32_t xxhash32(const char* input, const size_t& length, const uint32_t& seed) {

static constexpr uint32_t PRIME1 = 0x9E3779B1U;
static constexpr uint32_t PRIME2 = 0x85EBCA77U;
static constexpr uint32_t PRIME3 = 0xC2B2AE3DU;
static constexpr uint32_t PRIME4 = 0x27D4EB2FU;
static constexpr uint32_t PRIME5 = 0x165667B1U;

// Cast the input pointer to a character pointer.
const auto* input = static_cast<const char*>(input_v);
/// The hash value being calculated.
uint32_t h{};
/// A pointer to the current position in the input buffer.
Expand Down Expand Up @@ -156,16 +154,14 @@ namespace util {
return h;
}

uint64_t xxhash64(const void* input_v, const size_t& length, const uint64_t& seed) {
uint64_t xxhash64(const char* input, const size_t& length, const uint64_t& seed) {

static constexpr uint64_t PRIME1 = 11400714785074694791ULL;
static constexpr uint64_t PRIME2 = 14029467366897019727ULL;
static constexpr uint64_t PRIME3 = 1609587929392839161ULL;
static constexpr uint64_t PRIME4 = 9650029242287828579ULL;
static constexpr uint64_t PRIME5 = 2870177450012600261ULL;

// Cast the input pointer to a character pointer.
const auto* input = static_cast<const char*>(input_v);
/// The hash value being calculated.
uint64_t h{};
/// A pointer to the current position in the input buffer.
Expand Down
22 changes: 16 additions & 6 deletions src/util/serialise/xxhash.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,30 +31,40 @@ namespace util {
namespace serialise {

/**
* Calculates the 32-bit xxHash of the input data.
* Calculates the 32-bit xxhash of the input data.
*
* This function calculates the 32-bit xxHash of the input data using the specified seed value.
* This function calculates the 32-bit xxhash of the input data using the specified seed value.
*
* @param input Pointer to the input data.
* @param length Length of the input data in bytes.
* @param seed Seed value to use for the hash calculation. Default value is 0.
*
* @return The 32-bit xxHash of the input data.
*/
uint32_t xxhash32(const void* input, const size_t& length, const uint32_t& seed = 0);
uint32_t xxhash32(const char* input, const size_t& length, const uint32_t& seed = 0);

template <typename T>
uint32_t xxhash32(const T* input, const uint32_t& seed = 0) {
return xxhash32(reinterpret_cast<const char*>(input), sizeof(T), seed);
}

/**
* Calculates the 64-bit xxHash of the input data.
* Calculates the 64-bit xxhash of the input data.
*
* This function calculates the 64-bit xxHash of the input data using the specified seed value.
* This function calculates the 64-bit xxhash of the input data using the specified seed value.
*
* @param input Pointer to the input data.
* @param length Length of the input data in bytes.
* @param seed Seed value to use for the hash calculation. Default value is 0.
*
* @return The 64-bit xxHash of the input data.
*/
uint64_t xxhash64(const void* input, const size_t& length, const uint64_t& seed = 0);
uint64_t xxhash64(const char* input, const size_t& length, const uint64_t& seed = 0);

template <typename T>
uint64_t xxhash64(const T* input, const uint64_t& seed = 0) {
return xxhash64(reinterpret_cast<const char*>(input), sizeof(T), seed);
}

} // namespace serialise
} // namespace util
Expand Down
4 changes: 3 additions & 1 deletion src/util/string_join.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,9 @@ namespace util {

namespace detail {
// No argument base case
inline void do_string_join(std::stringstream& /*out*/, const std::string& /*delimiter*/) {}
inline void do_string_join(std::stringstream& /*out*/, const std::string& /*delimiter*/) {
// Terminating case with no arguments, do nothing
}

// Single argument case
template <typename Last>
Expand Down
Loading