refactor: add explicit comparison to strcmp/stricmp results (bugprone-suspicious-string-compare)

Author: bobtista

.clang-tidy

@@ -0,0 +1,167 @@
+# TheSuperHackers @build JohnsterID 15/09/2025 Add clang-tidy configuration for code quality analysis
+---
+# Clang-tidy configuration for GeneralsGameCode project
+# This configuration is tailored for a legacy C++98/C++20 hybrid codebase
+# with Windows-specific code and COM interfaces
+
+# Enable specific checks that are appropriate for this codebase
+Checks: >
+  -*,
+  bugprone-*,
+  -bugprone-easily-swappable-parameters,
+  -bugprone-implicit-widening-of-multiplication-result,
+  -bugprone-narrowing-conversions,
+  -bugprone-signed-char-misuse,
+  cert-*,
+  -cert-dcl21-cpp,
+  -cert-dcl50-cpp,
+  -cert-dcl58-cpp,
+  -cert-env33-c,
+  -cert-err58-cpp,
+  clang-analyzer-*,
+  -clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling,
+  cppcoreguidelines-*,
+  -cppcoreguidelines-avoid-c-arrays,
+  -cppcoreguidelines-avoid-magic-numbers,
+  -cppcoreguidelines-avoid-non-const-global-variables,
+  -cppcoreguidelines-init-variables,
+  -cppcoreguidelines-macro-usage,
+  -cppcoreguidelines-no-malloc,
+  -cppcoreguidelines-owning-memory,
+  -cppcoreguidelines-pro-bounds-array-to-pointer-decay,
+  -cppcoreguidelines-pro-bounds-constant-array-index,
+  -cppcoreguidelines-pro-bounds-pointer-arithmetic,
+  -cppcoreguidelines-pro-type-cstyle-cast,
+  -cppcoreguidelines-pro-type-reinterpret-cast,
+  -cppcoreguidelines-pro-type-union-access,
+  -cppcoreguidelines-pro-type-vararg,
+  -cppcoreguidelines-special-member-functions,
+  google-*,
+  -google-build-using-namespace,
+  -google-explicit-constructor,
+  -google-readability-casting,
+  -google-readability-todo,
+  -google-runtime-int,
+  -google-runtime-references,
+  hicpp-*,
+  -hicpp-avoid-c-arrays,
+  -hicpp-explicit-conversions,
+  -hicpp-no-array-decay,
+  -hicpp-signed-bitwise,
+  -hicpp-special-member-functions,
+  -hicpp-uppercase-literal-suffix,
+  -hicpp-use-auto,
+  -hicpp-vararg,
+  misc-*,
+  -misc-const-correctness,
+  -misc-include-cleaner,
+  -misc-non-private-member-variables-in-classes,
+  -misc-use-anonymous-namespace,
+  modernize-*,
+  -modernize-avoid-c-arrays,
+  -modernize-concat-nested-namespaces,
+  -modernize-loop-convert,
+  -modernize-pass-by-value,
+  -modernize-raw-string-literal,
+  -modernize-return-braced-init-list,
+  -modernize-use-auto,
+  -modernize-use-default-member-init,
+  -modernize-use-nodiscard,
+  -modernize-use-trailing-return-type,
+  performance-*,
+  -performance-avoid-endl,
+  portability-*,
+  readability-*,
+  -readability-avoid-const-params-in-decls,
+  -readability-braces-around-statements,
+  -readability-convert-member-functions-to-static,
+  -readability-function-cognitive-complexity,
+  -readability-identifier-length,
+  -readability-implicit-bool-conversion,
+  -readability-isolate-declaration,
+  -readability-magic-numbers,
+  -readability-named-parameter,
+  -readability-redundant-access-specifiers,
+  -readability-uppercase-literal-suffix
+
+# Treat warnings as errors for CI/CD
+WarningsAsErrors: false
+
+# Header filter to include project headers
+HeaderFilterRegex: '(Core|Generals|GeneralsMD|Dependencies)/.*\.(h|hpp)$'
+
+# Check options for specific rules
+CheckOptions:
+  # Naming conventions - adapted for the existing codebase style
+  - key: readability-identifier-naming.ClassCase
+    value: CamelCase
+  - key: readability-identifier-naming.StructCase
+    value: CamelCase
+  - key: readability-identifier-naming.FunctionCase
+    value: CamelCase
+  - key: readability-identifier-naming.MethodCase
+    value: CamelCase
+  - key: readability-identifier-naming.VariableCase
+    value: lower_case
+  - key: readability-identifier-naming.ParameterCase
+    value: lower_case
+  - key: readability-identifier-naming.MemberCase
+    value: lower_case
+  - key: readability-identifier-naming.MemberPrefix
+    value: m_
+  - key: readability-identifier-naming.ConstantCase
+    value: UPPER_CASE
+  - key: readability-identifier-naming.EnumConstantCase
+    value: UPPER_CASE
+  - key: readability-identifier-naming.MacroDefinitionCase
+    value: UPPER_CASE
+
+  # Performance settings
+  - key: performance-for-range-copy.WarnOnAllAutoCopies
+    value: true
+  - key: performance-unnecessary-value-param.AllowedTypes
+    value: 'AsciiString;UnicodeString;Utf8String;Utf16String'
+
+  # Modernize settings - be conservative for legacy code
+  - key: modernize-use-nullptr.NullMacros
+    value: 'NULL'
+  - key: modernize-replace-auto-ptr.IncludeStyle
+    value: llvm
+
+  # Readability settings
+  - key: readability-function-size.LineThreshold
+    value: 150
+  - key: readability-function-size.StatementThreshold
+    value: 100
+  - key: readability-function-size.BranchThreshold
+    value: 25
+  - key: readability-function-size.ParameterThreshold
+    value: 8
+  - key: readability-function-size.NestingThreshold
+    value: 6
+
+  # Bugprone settings
+  - key: bugprone-argument-comment.StrictMode
+    value: false
+  - key: bugprone-suspicious-string-compare.WarnOnImplicitComparison
+    value: true
+  - key: bugprone-suspicious-string-compare.WarnOnLogicalNotComparison
+    value: true
+
+  # Google style settings
+  - key: google-readability-braces-around-statements.ShortStatementLines
+    value: 2
+  - key: google-readability-function-size.StatementThreshold
+    value: 100
+
+  # CERT settings
+  - key: cert-dcl16-c.NewSuffixes
+    value: 'L;LL;LU;LLU'
+  - key: cert-oop54-cpp.WarnOnlyIfThisHasSuspiciousField
+    value: false
+
+# Use .clang-format for formatting suggestions
+FormatStyle: file
+
+# Exclude certain directories and files
+# Note: This is handled by HeaderFilterRegex above, but can be extended

Core/GameEngine/Source/Common/System/GameMemory.cpp

@@ -2681,7 +2681,7 @@ MemoryPool *MemoryPoolFactory::findMemoryPool(const char *poolName)
 {
 	for (MemoryPool *pool = m_firstPoolInFactory; pool; pool = pool->getNextPoolInList())
 	{
-		if (!strcmp(poolName, pool->getPoolName()))
+		if (strcmp(poolName, pool->getPoolName()) == 0)
 		{
 			DEBUG_ASSERTCRASH(poolName == pool->getPoolName(), ("hmm, ptrs should probably match here"));
 			return pool;

Core/GameEngine/Source/GameNetwork/GameSpy/PeerDefs.cpp

@@ -340,7 +340,7 @@ void GameSpyInfo::addGroupRoom( GameSpyGroupRoom room )
 		groupLabel.format("GUI:%s", room.m_name.str());
 		room.m_translatedName = TheGameText->fetch(groupLabel);
 		m_groupRooms[room.m_groupID] = room;
-		if ( !stricmp("quickmatch", room.m_name.str()) )
+		if ( stricmp("quickmatch", room.m_name.str()) == 0 )
 		{
 			DEBUG_LOG(("Group room %d (%s) is the QuickMatch room", room.m_groupID, room.m_name.str()));
 			TheGameSpyConfig->setQMChannel(room.m_groupID);

Core/GameEngine/Source/GameNetwork/GameSpy/Thread/PeerThread.cpp

@@ -471,7 +471,7 @@ Int PeerThreadClass::findServer( SBServer server )
 			UnsignedInt oldPrivateIP = SBServerGetPrivateInetAddress(it->second);
 			UnsignedShort oldPrivatePort = SBServerGetPrivateQueryPort(it->second);
 			UnsignedInt oldPublicIP = SBServerGetPublicInetAddress(it->second);
-			if (!strcmp(oldName, newName) &&
+			if (strcmp(oldName, newName) == 0 &&
 				oldPrivateIP == newPrivateIP &&
 				oldPublicIP == newPublicIP &&
 				oldPrivatePort == newPrivatePort)
@@ -1842,7 +1842,7 @@ void PeerThreadClass::handleQMMatch(PEER peer, Int mapIndex, Int seed,
 		Int i=0;
 		for (; i<MAX_SLOTS; ++i)
 		{
-			if (playerName[i] && stricmp(playerName[i], m_loginName.c_str()))
+			if (playerName[i] && stricmp(playerName[i], m_loginName.c_str()) != 0)
 			{
 				peerMessagePlayer( peer, playerName[i], "We're matched!", NormalMessage );
 			}
@@ -2705,7 +2705,7 @@ void playerLeftCallback(PEER peer, RoomType roomType, const char * nick, const c
 
 	if (t->getQMStatus() != QM_IDLE && t->getQMStatus() != QM_STOPPED)
 	{
-		if (!stricmp(t->getQMBotName().c_str(), nick))
+		if (stricmp(t->getQMBotName().c_str(), nick) == 0)
 		{
 			// matchbot left - bail
 			PeerResponse resp;
@@ -2845,9 +2845,9 @@ static void listingGamesCallback(PEER peer, PEERBool success, const char * name,
 		DEBUG_LOG(("Game name is '%s'", name));
 		const char *newname = SBServerGetStringValue(server, "gamename", (char *)name);
 #if RTS_GENERALS
-		if (strcmp(newname, "ccgenerals"))
+		if (strcmp(newname, "ccgenerals") != 0)
 #elif RTS_ZEROHOUR
-		if (strcmp(newname, "ccgenzh"))
+		if (strcmp(newname, "ccgenzh") != 0)
 #endif
 			name = newname;
 		DEBUG_LOG(("Game name is now '%s'", name));

Core/GameEngine/Source/GameNetwork/NAT.cpp

@@ -1169,7 +1169,7 @@ void NAT::processGlobalMessage(Int slotNum, const char *options) {
 		++ptr;
 	}
 	DEBUG_LOG(("NAT::processGlobalMessage - got message from slot %d, message is \"%s\"", slotNum, ptr));
-	if (!strncmp(ptr, "PROBED", strlen("PROBED"))) {
+	if (strncmp(ptr, "PROBED", strlen("PROBED")) == 0) {
 		// format: PROBED<node number>
 		// a probe has been sent at us, if we are waiting because of a netgear or something, we
 		// should start sending our own probes.
@@ -1180,7 +1180,7 @@ void NAT::processGlobalMessage(Int slotNum, const char *options) {
 		} else {
 			DEBUG_LOG(("NAT::processGlobalMessage - probed by node %d, not our target", node));
 		}
-	} else if (!strncmp(ptr, "CONNDONE", strlen("CONNDONE"))) {
+	} else if (strncmp(ptr, "CONNDONE", strlen("CONNDONE")) == 0) {
 		// format: CONNDONE<node number>
 		// we should get the node number of the player who's connection is done from the options
 		// and mark that down as part of the connectionStates.
@@ -1205,7 +1205,7 @@ void NAT::processGlobalMessage(Int slotNum, const char *options) {
 		} else {
 			DEBUG_LOG(("NAT::processGlobalMessage - got a connection done message that isn't from this round. node: %d sending node: %d", node, sendingNode));
 		}
-	} else if (!strncmp(ptr, "CONNFAILED", strlen("CONNFAILED"))) {
+	} else if (strncmp(ptr, "CONNFAILED", strlen("CONNFAILED")) == 0) {
 		// format: CONNFAILED<node number>
 		// we should get the node number of the player who's connection failed from the options
 		// and mark that down as part of the connectionStates.
@@ -1214,7 +1214,7 @@ void NAT::processGlobalMessage(Int slotNum, const char *options) {
 			DEBUG_LOG(("NAT::processGlobalMessage - node %d's connection failed, setting connection state to failed", node));
 			setConnectionState(node, NATCONNECTIONSTATE_FAILED);
 		}
-	} else if (!strncmp(ptr, "PORT", strlen("PORT"))) {
+	} else if (strncmp(ptr, "PORT", strlen("PORT")) == 0) {
 		// format: PORT<node number> <port number> <internal IP>
 		// we should get the node number and the mangled port number of the client we
 		// are supposed to be communicating with and start probing them. No, that was not

Core/GameEngineDevice/Source/StdDevice/Common/StdLocalFileSystem.cpp

@@ -241,7 +241,7 @@ void StdLocalFileSystem::getFileListInDirectory(const AsciiString& currentDirect
 	while (!done)	{
 		std::string filenameStr = iter->path().filename().string();
 		if (!iter->is_directory() && iter->path().extension() == searchExt &&
-			(strcmp(filenameStr.c_str(), ".") && strcmp(filenameStr.c_str(), ".."))) {
+			(strcmp(filenameStr.c_str(), ".") != 0 && strcmp(filenameStr.c_str(), ".."))) {
 			// if we haven't already, add this filename to the list.
 			// a stl set should only allow one copy of each filename
 			AsciiString newFilename = iter->path().string().c_str();
@@ -268,7 +268,7 @@ void StdLocalFileSystem::getFileListInDirectory(const AsciiString& currentDirect
 		while (!done) {
 			std::string filenameStr = iter->path().filename().string();
 			if(iter->is_directory() &&
-				(strcmp(filenameStr.c_str(), ".") && strcmp(filenameStr.c_str(), ".."))) {
+				(strcmp(filenameStr.c_str(), ".") != 0 && strcmp(filenameStr.c_str(), ".."))) {
 				AsciiString tempsearchstr(filenameStr.c_str());
 
 				// recursively add files in subdirectories if required.

Core/GameEngineDevice/Source/Win32Device/Common/Win32LocalFileSystem.cpp

@@ -139,7 +139,7 @@ void Win32LocalFileSystem::getFileListInDirectory(const AsciiString& currentDire
 
 	while (!done)	{
 		if (!(findData.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) &&
-				(strcmp(findData.cFileName, ".") && strcmp(findData.cFileName, ".."))) {
+				(strcmp(findData.cFileName, ".") != 0 && strcmp(findData.cFileName, ".."))) {
 			// if we haven't already, add this filename to the list.
 				// a stl set should only allow one copy of each filename
 				AsciiString newFilename;
@@ -165,7 +165,7 @@ void Win32LocalFileSystem::getFileListInDirectory(const AsciiString& currentDire
 
 		while (!done) {
 			if ((findData.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) &&
-					(strcmp(findData.cFileName, ".") && strcmp(findData.cFileName, ".."))) {
+					(strcmp(findData.cFileName, ".") != 0 && strcmp(findData.cFileName, ".."))) {
 
 					AsciiString tempsearchstr;
 					tempsearchstr.concat(currentDirectory);

Core/Libraries/Source/WWVegas/WWDownload/Download.cpp

@@ -47,7 +47,7 @@ HRESULT CDownload::DownloadFile(LPCSTR server, LPCSTR username, LPCSTR password,
 	// If we're still connected, make sure we're on the right server
 	if (m_Status == DOWNLOADSTATUS_FINDINGFILE)
 	{
-		if ((strcmp(m_Server, server)) || (strcmp(m_Login, username)))
+		if ((strcmp(m_Server, server) != 0) || (strcmp(m_Login, username)))
 		{
 			// Damn, a server switch.  Close conn & fix state
 			m_Ftp->DisconnectFromServer();

Core/Libraries/Source/WWVegas/WWLib/argv.cpp

@@ -103,15 +103,15 @@ const char *ArgvClass::Find_Again(const char *arg)
 			if (Is_Exact_Size()) {
 				// Case Sensitive, Exact Size.
 				for (; CurrentPos < Argc; CurrentPos++) {
-					if (!strcmp(arg, Argv[CurrentPos])) {
+					if (strcmp(arg, Argv[CurrentPos]) == 0) {
 						return Argv[CurrentPos];
 					}
 				}
 			} else {
 				// Case Sensitive, Match first strlen(arg).
 				int len = strlen(arg);
 				for (; CurrentPos < Argc; CurrentPos++) {
-					if (!strncmp(arg, Argv[CurrentPos], len)) {
+					if (strncmp(arg, Argv[CurrentPos], len) == 0) {
 						return Argv[CurrentPos];
 					}
 				}
@@ -120,15 +120,15 @@ const char *ArgvClass::Find_Again(const char *arg)
 			if (Is_Exact_Size()) {
 				// Note case sensitive, Exact Size.
 				for (; CurrentPos < Argc; CurrentPos++) {
-					if (!stricmp(arg, Argv[CurrentPos])) {
+					if (stricmp(arg, Argv[CurrentPos]) == 0) {
 						return Argv[CurrentPos];
 					}
 				}
 			} else {
 				// Note case sensitive, Match first strlen(arg).
 				int len = strlen(arg);
 				for (; CurrentPos < Argc; CurrentPos++) {
-					if (!strnicmp(arg, Argv[CurrentPos], len)) {
+					if (strnicmp(arg, Argv[CurrentPos], len) == 0) {
 						return Argv[CurrentPos];
 					}
 				}
@@ -201,7 +201,7 @@ int ArgvClass::Init(char *lpCmdLine, const char *fileprefix)
 		bool was_file = false;
 
 		// See if we are to load a file with parameters in it.
-		if (fp_cmp_len && !strncmp(fileprefix, ptr, fp_cmp_len)) {
+		if (fp_cmp_len && strncmp(fileprefix, ptr, fp_cmp_len) == 0) {
 			ptr += fp_cmp_len;
 			if (*ptr) {
 				was_file = Load_File(ptr);

Core/Libraries/Source/WWVegas/WWLib/cpudetect.cpp

@@ -1113,7 +1113,7 @@ void CPUDetectClass::Init_Compact_Log()
 	Get_OS_Info(os_info,OSVersionPlatformId,OSVersionNumberMajor,OSVersionNumberMinor,OSVersionBuildNumber);
 	COMPACTLOG(("%s\t",os_info.Code));
 
-	if (!stricmp(os_info.SubCode,"UNKNOWN")) {
+	if (stricmp(os_info.SubCode,"UNKNOWN") == 0) {
 		COMPACTLOG(("%d\t",OSVersionBuildNumber&0xffff));
 	}
 	else {