Add clang-tidy support for static code analysis (#2648)

This PR allows static code analysis using clang-tidy, suggested by #2616.

Clang has more limited `constexpr` support than GCC so cannot parse some of the FlashString and NanoTime code without modification. However, these changes are only made when `__clang__` is defined and do not affect regular builds with GCC.

This PR also includes some basic code fixes which clang identifies. There are a lot more to look at.

To try it out, build a project with:

```
make CLANG_TIDY=clang-tidy
```

Add additional parameters like this:

```
make CLANG_TIDY="clang-tidy --fix --fix-errors"
```

Notes:

- Don't use `-j` option as clang-tidy output and fixes don't get serialised correctly.
- Settings are in the main `.clang-tidy` file. A custom version can be used and passed on the command line.
- I've been using clang 17.0.6 https://releases.llvm.org/17.0.1/tools/clang/tools/extra/docs/clang-tidy/index.html
- Only source files which haven't been built are inspected. So, to restrict which code gets processed built the entire application then 'clean' the relevant modules before proceeding with clang-tidy.
- No object code is generated by clang.

.clang-tidy

@@ -1,28 +1,48 @@
 ---
-Checks:          'clang-diagnostic-*,clang-analyzer-*,-clang-analyzer-alpha*'
-HeaderFilterRegex: ''
-AnalyzeTemporaryDtors: false
-User:            slavey
+Checks:
+  -*
+  bugprone-*
+  -bugprone-macro-parentheses
+  -bugprone-reserved-identifier
+  -bugprone-easily-swappable-parameters
+  clang-analyzer-*
+  performance-*
+  portability-*
+  cppcoreguidelines-*
+  -cppcoreguidelines-avoid-c-arrays
+  -cppcoreguidelines-avoid-do-while
+  -cppcoreguidelines-pro-bounds-array-to-pointer-decay
+  -cppcoreguidelines-macro-usage
+  -cppcoreguidelines-pro-type-reinterpret-cast
+  -cppcoreguidelines-pro-type-static-cast-downcast
+  -cppcoreguidelines-pro-type-vararg
+  -cppcoreguidelines-pro-bounds-pointer-arithmetic
+  -cppcoreguidelines-pro-bounds-constant-array-index
+  -cppcoreguidelines-pro-type-union-access
+  -cppcoreguidelines-avoid-const-or-ref-data-members
+  -cppcoreguidelines-non-private-member-variables-in-classes
+HeaderFilterRegex: '.*'
 CheckOptions:    
-  - key:             google-readability-braces-around-statements.ShortStatementLines
-    value:           '1'
-  - key:             google-readability-function-size.StatementThreshold
-    value:           '800'
-  - key:             google-readability-namespace-comments.ShortNamespaceLines
-    value:           '10'
-  - key:             google-readability-namespace-comments.SpacesBeforeComments
-    value:           '2'
-  - key:             modernize-loop-convert.MaxCopySize
-    value:           '16'
-  - key:             modernize-loop-convert.MinConfidence
-    value:           reasonable
-  - key:             modernize-loop-convert.NamingStyle
-    value:           CamelCase
-  - key:             modernize-pass-by-value.IncludeStyle
-    value:           llvm
-  - key:             modernize-replace-auto-ptr.IncludeStyle
-    value:           llvm
-  - key:             modernize-use-nullptr.NullMacros
-    value:           'NULL'
+  - key:   google-readability-braces-around-statements.ShortStatementLines
+    value: '1'
+  - key:   google-readability-function-size.StatementThreshold
+    value: '800'
+  - key:   google-readability-namespace-comments.ShortNamespaceLines
+    value: '10'
+  - key:   google-readability-namespace-comments.SpacesBeforeComments
+    value: '2'
+  - key:   modernize-loop-convert.MaxCopySize
+    value: '16'
+  - key:   modernize-loop-convert.MinConfidence
+    value: reasonable
+  - key:   modernize-loop-convert.NamingStyle
+    value: CamelCase
+  - key:   modernize-pass-by-value.IncludeStyle
+    value: llvm
+  - key:   modernize-replace-auto-ptr.IncludeStyle
+    value: llvm
+  - key:   cppcoreguidelines-pro-type-static-cast-downcast.StrictMode
+    value: false
+  - key:   cppcoreguidelines-explicit-virtual-functions.IgnoreDestructors
+    value: true
 ...
-

Sming/Arch/Esp32/Core/Interrupts.cpp

@@ -89,7 +89,7 @@ void attachInterrupt(uint8_t pin, InterruptDelegate delegateFunction, GPIO_INT_T
 		return; // WTF o_O
 	}
 	gpioInterruptsList[pin] = nullptr;
-	delegateFunctionList[pin] = delegateFunction;
+	delegateFunctionList[pin] = std::move(delegateFunction);
 	attachInterruptHandler(pin, type);
 }
 

Sming/Arch/Esp8266/Core/Interrupts.cpp

@@ -73,7 +73,7 @@ void attachInterrupt(uint8_t pin, InterruptDelegate delegateFunction, GPIO_INT_T
 		return; // WTF o_O
 	}
 	gpioInterruptsList[pin] = nullptr;
-	delegateFunctionList[pin] = delegateFunction;
+	delegateFunctionList[pin] = std::move(delegateFunction);
 	attachInterruptHandler(pin, type);
 }
 

Sming/Arch/Host/Components/driver/hw_timer.cpp

@@ -120,7 +120,6 @@ class CTimerThread : public CThread
 private:
 	typedef std::ratio<HW_TIMER_BASE_CLK, 1000000> base_ticks_per_us;
 	uint32_t divisor = 1;
-	uint32_t frequency = HW_TIMER_BASE_CLK;
 	uint64_t start_time = 0;
 	uint64_t interval = 0; // In microseconds
 	CSemaphore sem;		   // Signals state change
@@ -130,7 +129,6 @@ class CTimerThread : public CThread
 	State state = stopped;
 
 	hw_timer_source_type_t source_type = TIMER_FRC1_SOURCE;
-	unsigned irq_level = 1;
 	struct {
 		hw_timer_callback_t func = nullptr;
 		void* arg = nullptr;

Sming/Arch/Host/Components/driver/uart.cpp

@@ -73,11 +73,6 @@ void notify(smg_uart_t* uart, smg_uart_notify_code_t code)
 	}
 }
 
-__forceinline bool smg_uart_isr_enabled(uint8_t nr)
-{
-	return bitRead(isrMask, nr);
-}
-
 } // namespace
 
 smg_uart_t* smg_uart_get_uart(uint8_t uart_nr)

Sming/Arch/Host/Components/esp_hal/clk.c

@@ -11,16 +11,13 @@ static uint32_t base_ccount;
  */
 static uint32_t get_ccount(uint64_t nanos)
 {
-	uint32_t ccount;
 	if(base_nanos == 0) {
 		base_nanos = nanos;
 		base_ccount = nanos / cpu_frequency;
-		ccount = base_ccount;
-	} else {
-		ccount = base_ccount + cpu_frequency * ((nanos - base_nanos) / 1000);
+		return base_ccount;
 	}
 
-	return ccount;
+	return base_ccount + cpu_frequency * ((nanos - base_nanos) / 1000);
 }
 
 bool system_update_cpu_freq(uint8_t freq)

Sming/Arch/Host/Components/esp_hal/system.cpp

@@ -27,7 +27,7 @@ static uint64_t initTime()
 	timeref.startTicks = os_get_nanoseconds();
 #endif
 
-	timeval tv;
+	timeval tv{};
 	gettimeofday(&tv, nullptr);
 	return (1000000ULL * tv.tv_sec) + tv.tv_usec;
 }
@@ -41,7 +41,7 @@ uint64_t os_get_nanoseconds()
 	QueryPerformanceCounter(&count);
 	return timeref.countsPerNanosecond * uint64_t(count.QuadPart - timeref.startCount.QuadPart);
 #else
-	timespec ts;
+	timespec ts{};
 	clock_gettime(CLOCK_MONOTONIC, &ts);
 	return (1000000000ULL * ts.tv_sec) + ts.tv_nsec - timeref.startTicks;
 #endif

Sming/Arch/Host/Components/esp_hal/tasks.cpp

@@ -9,11 +9,8 @@ class TaskQueue
 {
 public:
 	TaskQueue(os_task_t callback, os_event_t* events, uint8_t length)
+		: callback(callback), events(events), length(length)
 	{
-		this->callback = callback;
-		this->events = events;
-		this->length = length;
-		read = count = 0;
 	}
 
 	bool post(os_signal_t sig, os_param_t par)
@@ -45,8 +42,8 @@ class TaskQueue
 	static CMutex mutex;
 	os_task_t callback;
 	os_event_t* events;
-	uint8_t read;
-	uint8_t count;
+	uint8_t read{0};
+	uint8_t count{0};
 	uint8_t length;
 };
 

Sming/Arch/Host/Components/hostlib/hostlib.c

@@ -35,14 +35,12 @@ size_t getHostAppDir(char* path, size_t bufSize)
 		return 0;
 	}
 
-	size_t len;
-	char sep;
 #ifdef __WIN32
-	len = GetModuleFileName(NULL, path, bufSize);
-	sep = '\\';
+	size_t len = GetModuleFileName(NULL, path, bufSize);
+	char sep = '\\';
 #else
-	len = readlink("/proc/self/exe", path, bufSize - 1);
-	sep = '/';
+	size_t len = readlink("/proc/self/exe", path, bufSize - 1);
+	char sep = '/';
 #endif
 	path[len] = '\0';
 	char* p = strrchr(path, sep);

Sming/Arch/Host/Components/hostlib/hostmsg.c

@@ -56,7 +56,7 @@ void host_printf(const char* fmt, ...)
 
 void host_printfp(const char* fmt, const char* pretty_function, ...)
 {
-	size_t len;
+	size_t len = 0;
 	const char* name = get_method_name(pretty_function, &len);
 
 	va_list args;

Sming/Arch/Host/Components/hostlib/keyb.cpp

@@ -34,9 +34,14 @@
 #include <fcntl.h>
 #include <termios.h>
 
-static bool g_orig_values_saved, g_values_changed;
-static struct termios g_orig_attr;
-static int g_orig_flags;
+namespace
+{
+bool g_orig_values_saved;
+bool g_values_changed;
+struct termios g_orig_attr;
+int g_orig_flags;
+
+} // namespace
 
 #endif
 
@@ -62,9 +67,9 @@ class CKeycode
 	int add(int c);
 
 private:
-	char m_esc;
-	char m_buffer[32];
-	unsigned m_count;
+	char m_esc{};
+	char m_buffer[32]{};
+	unsigned m_count{};
 
 	void push(char c)
 	{
@@ -228,14 +233,16 @@ int CKeycode::add(int c)
 void keyb_restore()
 {
 #ifndef __WIN32
-	if(g_values_changed) {
-		static struct termios attr = g_orig_attr;
-		attr.c_lflag |= ICANON | ECHO;
-		tcsetattr(STDIN_FILENO, TCSANOW, &attr);
-
-		(void)fcntl(0, F_SETFL, g_orig_flags);
-		g_values_changed = false;
+	if(!g_values_changed) {
+		return;
 	}
+
+	static struct termios attr = g_orig_attr;
+	attr.c_lflag |= ICANON | ECHO;
+	tcsetattr(STDIN_FILENO, TCSANOW, &attr);
+
+	(void)fcntl(0, F_SETFL, g_orig_flags);
+	g_values_changed = false;
 #endif
 }
 
@@ -271,14 +278,16 @@ int getch()
 int getkey()
 {
 	static CKeycode kc;
-	int c;
+	int c = 0;
 	for(;;) {
 		c = getch();
-		if(c == KEY_NONE)
+		if(c == KEY_NONE) {
 			break;
+		}
 		c = kc.add(c);
-		if(c != KEY_NONE)
+		if(c != KEY_NONE) {
 			break;
+		}
 	}
 	return c;
 }

Sming/Arch/Host/Components/hostlib/sockets.cpp

@@ -123,13 +123,12 @@ std::string socket_strerror()
 {
 	char buf[256];
 	buf[0] = '\0';
-	int ErrorCode;
 #ifdef __WIN32
-	ErrorCode = WSAGetLastError();
+	int ErrorCode = WSAGetLastError();
 	FormatMessageA(FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS | FORMAT_MESSAGE_ARGUMENT_ARRAY, nullptr,
 				   ErrorCode, 0, buf, sizeof(buf), nullptr);
 #else
-	ErrorCode = errno;
+	int ErrorCode = errno;
 	char* res = strerror_r(ErrorCode, buf, sizeof(buf));
 	if(res == nullptr) {
 		strcpy(buf, "Unknown");
@@ -438,7 +437,8 @@ CSocket* CServerSocket::try_connect()
 		return nullptr;
 	}
 
-	struct sockaddr sa;
+	struct sockaddr sa {
+	};
 	host_socklen_t len = sizeof(sa);
 	int fd = ::accept(m_fd, &sa, &len);
 	if(fd < 0) {

Sming/Arch/Host/Components/hostlib/sockets.h

@@ -58,7 +58,7 @@ class CSockAddr
 		struct sockaddr sa;
 		struct sockaddr_in in4;  // AF_INET
 		struct sockaddr_in6 in6; // AF_INET6
-	} m_addr;
+	} m_addr{};
 
 public:
 	CSockAddr()
@@ -113,12 +113,12 @@ class CSocket
 		assign(fd, addr);
 	}
 
-	virtual ~CSocket()
+	~CSocket()
 	{
 		close();
 	}
 
-	virtual void close();
+	void close();
 
 	bool setblocking(bool block);
 	bool bind(const CSockAddr& sa);
@@ -213,11 +213,16 @@ class CSocketList : public std::vector<CSocket*>
 class CServerSocket : public CSocket
 {
 public:
-	CServerSocket(int type = SOCK_STREAM) : CSocket(type), m_max_connections(1)
+	CServerSocket(int type = SOCK_STREAM) : CSocket(type)
 	{
 	}
 
-	void close() override
+	virtual ~CServerSocket()
+	{
+		close();
+	}
+
+	void close()
 	{
 		m_clients.closeall();
 		CSocket::close();
@@ -235,6 +240,6 @@ class CServerSocket : public CSocket
 	}
 
 private:
-	unsigned m_max_connections;
+	unsigned m_max_connections{1};
 	CSocketList m_clients;
 };

Sming/Arch/Host/Components/hostlib/startup.cpp

@@ -86,7 +86,7 @@ static size_t parse_flash_size(const char* str)
 	if(str == nullptr) {
 		return 0;
 	}
-	char* tail;
+	char* tail = nullptr;
 	long res = strtol(str, &tail, 0);
 	if(res < 0) {
 		return 0;
@@ -133,21 +133,22 @@ int main(int argc, char* argv[])
 	struct Config {
 		int pause{-1};
 		int exitpause{-1};
-		int loopcount;
-		uint8_t cpulimit;
-		bool initonly;
+		int loopcount{};
+		uint8_t cpulimit{};
+		bool initonly{};
 		bool enable_network{true};
-		UartServer::Config uart;
-		FlashmemConfig flash;
+		UartServer::Config uart{};
+		FlashmemConfig flash{};
 #ifndef DISABLE_NETWORK
-		struct lwip_param lwip;
+		struct lwip_param lwip {
+		};
 #endif
 	};
 	static Config config{};
 
 	int uart_num{-1};
 	option_tag_t opt;
-	const char* arg;
+	const char* arg = nullptr;
 	while((opt = get_option(argc, argv, arg)) != opt_none) {
 		switch(opt) {
 		case opt_help: