Address some issues found by sonarqube

src/clock.cpp

@@ -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

src/clock.hpp

@@ -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.

src/dsl/operation/TypeBind.hpp

@@ -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(

src/dsl/word/Always.hpp

@@ -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
                 });

src/dsl/word/Every.hpp

@@ -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));
                 });
 

src/dsl/word/IO.hpp

@@ -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));
                 });
 

src/dsl/word/Idle.hpp

@@ -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);
             });
 

src/dsl/word/Network.hpp

@@ -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));
                 });
 

src/dsl/word/TCP.hpp

@@ -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);
                 });

src/dsl/word/UDP.hpp

@@ -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
@@ -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

src/dsl/word/Watchdog.hpp

@@ -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));
@@ -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));

src/extension/ChronoController.cpp

@@ -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;
                             });

src/extension/NetworkController.cpp

@@ -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);
             }
         });
 

src/extension/network/NUClearNetwork.cpp

@@ -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
@@ -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 && static_cast<int>((s->acked[i] & expected) == expected);
                                     }
 
                                     // The remote has received this entire packet we can erase our sender

src/threading/Reaction.cpp

@@ -51,7 +51,7 @@ namespace threading {
 
     void Reaction::unbind() {
         // Unbind
-        for (auto& u : unbinders) {
+        for (const auto& u : unbinders) {
             u(*this);
         }
     }

src/util/Logger.hpp

@@ -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...));
             }
         }
 

src/util/get_hostname.cpp

@@ -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;
 }
 

src/util/serialise/xxhash.cpp

@@ -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.

src/util/serialise/xxhash.hpp

@@ -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

src/util/string_join.hpp

@@ -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>