Refactor code style and fix parameter bugs

Remove const from value parameters across display.h function signatures to follow project conventions. Rename variables for clarity (e.g., pData → data, SYMBOLS → kSymbols). Fix bug in netif.cpp where gw should be gateway parameter. Refactor HandleList() in remoteconfig.cpp with improved variable naming, nullptr checks, bounds validation, and add necessary includes. Remove duplicate private access specifier in display.h.

Author: vanvught

lib-display/include/spi/display.h

@@ -105,17 +105,17 @@ class Display : public LcdDriver {
 #else
         printf("ST7789 ");
 #endif
-        printf("(%u,%u)\n", rows_, cols_);
+        printf("(%u,%u)\n", static_cast<unsigned>(rows_), static_cast<unsigned>(cols_));
     }
 
     void Cls() { FillColour(kColorBackground); }
 
-    void SetCursorPos(const uint32_t nCol, const uint32_t row) {
+    void SetCursorPos(uint32_t nCol, uint32_t row) {
         cursor_x_ = nCol * s_pFONT->kWidth;
         cursor_y_ = row * s_pFONT->kHeight;
     }
 
-    void PutChar(const int c) {
+    void PutChar(int c) {
         DrawChar(cursor_x_, cursor_y_, static_cast<char>(c), s_pFONT, kColorBackground, kColorForeground);
 
         cursor_x_ += s_pFONT->kWidth;
@@ -137,7 +137,7 @@ class Display : public LcdDriver {
 	    }
 	}
 
-    void ClearLine(const uint32_t nLine) {
+    void ClearLine(uint32_t nLine) {
         if (__builtin_expect((!(nLine <= rows_)), 0)) {
             return;
         }
@@ -151,7 +151,7 @@ class Display : public LcdDriver {
         SetCursorPos(0, (nLine - 1U));
     }
 
-    void TextLine(const uint32_t nLine, const char* pText, const uint32_t nLength) {
+    void TextLine(uint32_t nLine, const char* pText, const uint32_t nLength) {
         if (__builtin_expect((!(nLine <= rows_)), 0)) {
             return;
         }
@@ -162,18 +162,18 @@ class Display : public LcdDriver {
 
     void ClearEndOfLine() { clear_end_of_line_ = true; }
 
-    void Text(const char* pData, uint32_t nLength) {
-        if (nLength > cols_) {
-            nLength = cols_;
+    void Text(const char* data, uint32_t length) {
+        if (length > cols_) {
+            length = cols_;
         }
 
-        for (uint32_t i = 0; i < nLength; i++) {
-            PutChar(pData[i]);
+        for (uint32_t i = 0; i < length; i++) {
+            PutChar(data[i]);
         }
     }
 
-    int Write(const uint32_t nLine, const char* pText) {
-        const auto* p = pText;
+    int Write(uint32_t line, const char* text) {
+        const auto* p = text;
         int nCount = 0;
 
         const auto columns = static_cast<int>(cols_);
@@ -182,28 +182,28 @@ class Display : public LcdDriver {
             ++p;
         }
 
-        TextLine(nLine, pText, static_cast<uint8_t>(nCount));
+        TextLine(line, text, static_cast<uint8_t>(nCount));
 
         return nCount;
     }
 
-    int Printf(const uint8_t nLine, const char* format, ...) {
+    int Printf(uint8_t line, const char* format, ...) {
         char buffer[32];
 
         va_list arp;
 
         va_start(arp, format);
 
-        auto i = vsnprintf(buffer, sizeof(buffer) / sizeof(buffer[0]), format, arp);
+        const auto kSize = vsnprintf(buffer, sizeof(buffer) / sizeof(buffer[0]), format, arp);
 
         va_end(arp);
 
-        TextLine(nLine, buffer, static_cast<uint16_t>(i));
+        TextLine(line, buffer, static_cast<uint16_t>(kSize));
 
-        return i;
+        return kSize;
     }
 
-    void TextStatus(const char* pText) {
+    void TextStatus(const char* text) {
         SetCursorPos(0, static_cast<uint8_t>(rows_ - 1));
 
         for (uint32_t i = 0; i < (cols_ - 1); i++) {
@@ -212,7 +212,7 @@ class Display : public LcdDriver {
 
         SetCursorPos(0, static_cast<uint8_t>(rows_ - 1));
 
-        Write(rows_, pText);
+        Write(rows_, text);
     }
 
 	void TextStatus(const char* text, ansi::Colours::Colour colour) {
@@ -221,20 +221,20 @@ class Display : public LcdDriver {
 	}
 
     void Progress() {
-        static constexpr char SYMBOLS[] = {'/', '-', '\\', '|'};
+        static constexpr char kSymbols[] = {'/', '-', '\\', '|'};
         static uint32_t nSymbolsIndex;
 
         SetCursorPos(GetColumns() - 1U, GetRows() - 1U);
-        PutChar(SYMBOLS[nSymbolsIndex++]);
+        PutChar(kSymbols[nSymbolsIndex++]);
 
-        if (nSymbolsIndex >= sizeof(SYMBOLS)) {
+        if (nSymbolsIndex >= sizeof(kSymbols)) {
             nSymbolsIndex = 0;
         }
     }
 
-    void SetContrast(const uint8_t nContrast) { SetBackLight(nContrast); }
+    void SetContrast(uint8_t nContrast) { SetBackLight(nContrast); }
 
-    void SetSleep(const bool bSleep) {
+    void SetSleep(bool bSleep) {
         is_sleep_ = bSleep;
 
         EnableSleep(bSleep);
@@ -282,7 +282,6 @@ class Display : public LcdDriver {
    private:
     void SetSleepTimer(const bool bActive);
 
-   private:
     uint32_t cols_;
     uint32_t rows_;
     uint32_t sleep_timeout_{1000U * 60U * display::Defaults::kSleepTimeout};

lib-network/src/core/netif.cpp

@@ -183,7 +183,7 @@ void SetGw(network::ip4_addr_t gateway) {
 
 void SetAddr(network::ip4_addr_t ipaddr, network::ip4_addr_t netmask, network::ip4_addr_t gateway) {
     DEBUG_ENTRY();
-    DEBUG_PRINTF(IPSTR " " IPSTR " " IPSTR, IP2STR(ipaddr.addr), IP2STR(netmask.addr), IP2STR(gw.addr));
+    DEBUG_PRINTF(IPSTR " " IPSTR " " IPSTR, IP2STR(ipaddr.addr), IP2STR(netmask.addr), IP2STR(gateway.addr));
 
     auto change_reason = NetifReason::kNone;
     netif_ext_callback_args_t cb_args;

lib-remoteconfig/src/remoteconfig.cpp

@@ -23,6 +23,7 @@
  * THE SOFTWARE.
  */
 
+
 #if defined(DEBUG_REMOTECONFIG)
 #undef NDEBUG
 #endif
@@ -31,6 +32,7 @@
 #include <cstdio>
 #include <cstring>
 #include <cassert>
+#include <algorithm>
 
 #include "remoteconfig.h"
 #include "firmware/firmwareversion.h"
@@ -41,6 +43,7 @@
 #include "dmxnode_nodetype.h"
 #include "json/remoteconfigparams.h"
 #endif
+#include "common/utils/utils_array.h"
 #include "display.h"
 #include "configstore.h"
 #include "firmware/debug/debug_dump.h"
@@ -126,9 +129,9 @@ RemoteConfig::~RemoteConfig() {
 
 #if !defined(CONFIG_REMOTECONFIG_MINIMUM)
 #if defined(ENABLE_HTTPD)
-    if (http_daemon_ != nullptr) {
-        delete http_daemon_;
-    }
+
+    delete http_daemon_;
+
 #endif
     network::apps::mdns::ServiceRecordDelete(network::apps::mdns::Services::kConfig);
 #endif
@@ -235,7 +238,6 @@ void RemoteConfig::HandleUptime() {
 
     const auto kUptime = timing::UpTime();
     const auto kLength = snprintf(udp_buffer_, remoteconfig::udp::kBufferSize - 1, "uptime: %us\n", static_cast<unsigned int>(kUptime));
-
     network::udp::Send(handle_, reinterpret_cast<const uint8_t*>(udp_buffer_), static_cast<uint32_t>(kLength), ip_from_, remoteconfig::udp::kPort);
 
     DEBUG_EXIT();
@@ -245,8 +247,8 @@ void RemoteConfig::HandleUptime() {
 void RemoteConfig::HandleVersion() {
     DEBUG_ENTRY();
 
-    const auto* p = FirmwareVersion::Get()->GetPrint();
-    const auto kLength = snprintf(udp_buffer_, remoteconfig::udp::kBufferSize - 1, "version:%s\n", p);
+    const auto* print = FirmwareVersion::Get()->GetPrint();
+    const auto kLength = snprintf(udp_buffer_, remoteconfig::udp::kBufferSize - 1, "version:%s\n", print);
     network::udp::Send(handle_, reinterpret_cast<const uint8_t*>(udp_buffer_), static_cast<uint32_t>(kLength), ip_from_, remoteconfig::udp::kPort);
 
     DEBUG_EXIT();
@@ -256,28 +258,62 @@ void RemoteConfig::HandleList() {
     DEBUG_ENTRY();
 
     constexpr auto kCmdLength = kGet[static_cast<uint32_t>(remoteconfig::udp::get::Command::kList)].kLength;
+
     auto* list_response = &udp_buffer_[kCmdLength + 2U];
-    const auto kListResponseBufferLength = remoteconfig::udp::kBufferSize - (kCmdLength + 2U);
-    int32_t list_length;
+
+    constexpr auto kListResponseBufferLength = remoteconfig::udp::kBufferSize - (kCmdLength + 2U);
 
     uint8_t display_name[common::store::remoteconfig::kDisplayNameLength];
+
     ConfigStore::Instance().RemoteConfigCopyArray(display_name, &common::store::RemoteConfig::display_name);
-    display_name[common::store::remoteconfig::kDisplayNameLength - 1] = '\0'; // Just to be safe
+
+    display_name[common::store::remoteconfig::kDisplayNameLength - 1U] = '\0';
 
 #if !defined(CONFIG_REMOTECONFIG_MINIMUM)
-    const auto* const kNodeTypeName = dmxnode::GetNodeType(dmxnode::kNodeType);
+    const char* node_type_name = dmxnode::GetNodeType(dmxnode::kNodeType);
+
+    if (node_type_name == nullptr) {
+        node_type_name = "Unknown";
+    }
 #else
-    const auto* const kNodeTypeName = "Bootloader TFTP";
+    constexpr const char* node_type_name = "Bootloader TFTP";
 #endif
 
+    const auto kOutputIndex = static_cast<uint32_t>(output_);
+    assert(kOutputIndex < common::ArraySize(kOutput));
+
+    const char* output_name = kOutput[kOutputIndex];
+
+    if (output_name == nullptr) {
+        output_name = "Unknown";
+    }
+
+    int list_length;
+
     if (display_name[0] != '\0') {
-        list_length = snprintf(list_response, kListResponseBufferLength - 1, "" IPSTR ",%s,%s,%u,%s\n", IP2STR(network::GetPrimaryIp()), kNodeTypeName, kOutput[static_cast<uint32_t>(output_)], static_cast<unsigned int>(active_outputs_),
-                               display_name);
+        list_length =
+            snprintf(list_response, kListResponseBufferLength, IPSTR ",%s,%s,%u,%s\n", 
+				IP2STR(network::GetPrimaryIp()), 
+				node_type_name, 
+				output_name, 
+				static_cast<unsigned>(active_outputs_), 
+				reinterpret_cast<const char*>(display_name));
     } else {
-        list_length = snprintf(list_response, kListResponseBufferLength - 1, "" IPSTR ",%s,%s,%u\n", IP2STR(network::GetPrimaryIp()), kNodeTypeName, kOutput[static_cast<uint32_t>(output_)], static_cast<unsigned int>(active_outputs_));
+        list_length = snprintf(list_response, kListResponseBufferLength, IPSTR ",%s,%s,%u\n", 
+			IP2STR(network::GetPrimaryIp()), 
+			node_type_name, 
+			output_name, 
+			static_cast<unsigned>(active_outputs_));
     }
 
-    network::udp::Send(handle_, reinterpret_cast<const uint8_t*>(list_response), static_cast<uint32_t>(list_length), ip_from_, remoteconfig::udp::kPort);
+    if (list_length < 0) {
+        DEBUG_EXIT();
+        return;
+    }
+
+    const auto kBytesToSend = static_cast<uint32_t>(std::min<size_t>(static_cast<size_t>(list_length), kListResponseBufferLength - 1U));
+
+    network::udp::Send(handle_, reinterpret_cast<const uint8_t*>(list_response), kBytesToSend, ip_from_, remoteconfig::udp::kPort);
 
     DEBUG_EXIT();
 }
@@ -354,10 +390,6 @@ void RemoteConfig::HandleTftpGet() {
     DEBUG_EXIT();
 }
 
-namespace board {
-bool Reboot();
-} // namespace board
-
 void RemoteConfig::HandleReboot() {
     DEBUG_ENTRY();
     board::Reboot();