Merge #3242

3242: Enable most clang-tidy bugprone-*, misc-*, performance-* checks r=Jupeyy a=def-

Fixes #3134

## Checklist

- [ ] Tested the change ingame
- [ ] Provided screenshots if it is a visual change
- [ ] Tested in combination with possibly related configuration options
- [ ] Written a unit test if it works standalone, system.c especially
- [ ] Considered possible null pointers and out of bounds array indexing
- [ ] Changed no physics that affect existing maps
- [ ] Tested the change with [ASan+UBSan or valgrind's memcheck](https://github.com/ddnet/ddnet/#using-addresssanitizer--undefinedbehavioursanitizer-or-valgrinds-memcheck) (optional)


Co-authored-by: def <dennis@felsin9.de>

.clang-tidy

@@ -8,14 +8,29 @@
 #   Too annoying to always align for perfect padding
 # clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling,
 #   TODO: Requires C11 to fix
+# misc-unused-parameters
+#   TODO: Many changes
 
 Checks: >
   -*,
+  bugprone-*,
+  -bugprone-branch-clone,
+  -bugprone-incorrect-roundings,
+  -bugprone-integer-division,
+  -bugprone-macro-parentheses,
+  -bugprone-narrowing-conversions,
+  -bugprone-parent-virtual-call,
+  -bugprone-unhandled-self-assignment,
   clang-analyzer-*,
   -clang-analyzer-optin.cplusplus.UninitializedObject,
   -clang-analyzer-optin.cplusplus.VirtualCall,
   -clang-analyzer-optin.performance.Padding,
   -clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling,
+  misc-*,
+  -misc-non-private-member-variables-in-classes,
+  -misc-static-assert,
+  -misc-unused-parameters,
   modernize-loop-convert,
   readability-qualified-auto,
+  performance-*,
 HeaderFilterRegex: 'src/.*'

src/base/system.c

@@ -1928,7 +1928,7 @@ int net_unix_send(UNIXSOCKET sock, UNIXSOCKETADDR *addr, void *data, int size)
 
 void net_unix_set_addr(UNIXSOCKETADDR *addr, const char *path)
 {
-	mem_zero(addr, sizeof(addr));
+	mem_zero(addr, sizeof(*addr));
 	addr->sun_family = AF_UNIX;
 	str_copy(addr->sun_path, path, sizeof(addr->sun_path));
 }
@@ -2064,7 +2064,7 @@ int fs_storage_path(const char *appname, char *path, int max)
 #else
 	snprintf(path, max, "%s/.%s", home, appname);
 	for(i = strlen(home) + 2; path[i]; i++)
-		path[i] = tolower(path[i]);
+		path[i] = tolower((unsigned char)path[i]);
 #endif
 
 	return 0;
@@ -2710,7 +2710,7 @@ const char *str_find_nocase(const char *haystack, const char *needle)
 	{
 		const char *a = haystack;
 		const char *b = needle;
-		while(*a && *b && tolower(*a) == tolower(*b))
+		while(*a && *b && tolower((unsigned char)*a) == tolower((unsigned char)*b))
 		{
 			a++;
 			b++;

src/engine/client/backend_sdl.cpp

@@ -134,7 +134,7 @@ static bool Texture2DTo3D(void *pImageBuffer, int ImageWidth, int ImageHeight, i
 	Target3DImageWidth = ImageWidth / SplitCountWidth;
 	Target3DImageHeight = ImageHeight / SplitCountHeight;
 
-	size_t FullImageWidth = (size_t)(ImageWidth * ImageColorChannelCount);
+	size_t FullImageWidth = (size_t)ImageWidth * ImageColorChannelCount;
 
 	for(int Y = 0; Y < SplitCountHeight; ++Y)
 	{
@@ -144,8 +144,8 @@ static bool Texture2DTo3D(void *pImageBuffer, int ImageWidth, int ImageHeight, i
 			{
 				int DepthIndex = X + Y * SplitCountWidth;
 
-				size_t TargetImageFullWidth = (size_t)(Target3DImageWidth * ImageColorChannelCount);
-				size_t TargetImageFullSize = (size_t)(TargetImageFullWidth * (size_t)Target3DImageHeight);
+				size_t TargetImageFullWidth = (size_t)Target3DImageWidth * ImageColorChannelCount;
+				size_t TargetImageFullSize = (size_t)TargetImageFullWidth * Target3DImageHeight;
 				ptrdiff_t ImageOffset = (ptrdiff_t)(((size_t)Y * FullImageWidth * (size_t)Target3DImageHeight) + ((size_t)Y3D * FullImageWidth) + ((size_t)X * TargetImageFullWidth));
 				ptrdiff_t TargetImageOffset = (ptrdiff_t)(TargetImageFullSize * (size_t)DepthIndex + ((size_t)Y3D * TargetImageFullWidth));
 				mem_copy(((uint8_t *)pTarget3DImageData) + TargetImageOffset, ((uint8_t *)pImageBuffer) + (ptrdiff_t)(ImageOffset), TargetImageFullWidth);

src/engine/client/keynames.h

@@ -6,7 +6,7 @@
 
 #include <string.h>
 
-const char g_aaKeyStrings[512][20] =
+const char g_aaKeyStrings[512][20] = // NOLINT(misc-definitions-in-headers)
 {
 	"unknown",
 	"&1",

src/engine/client/opengl_sl.cpp

@@ -188,7 +188,7 @@ void CGLSLCompiler::ParseLine(std::string &Line, const char *pReadLine, int Type
 						++pBuff;
 					}
 
-					if(*pBuff && *pBuff == ' ' && *(pBuff + 1) && *(pBuff + 1) == 'i' && *(pBuff + 2) == 'n')
+					if(*pBuff == ' ' && *(pBuff + 1) && *(pBuff + 1) == 'i' && *(pBuff + 2) == 'n')
 					{
 						pBuff += 3;
 						Line.append("attribute");

src/engine/client/serverbrowser.cpp

@@ -503,9 +503,9 @@ CServerBrowser::CServerEntry *CServerBrowser::Add(const NETADDR &Addr)
 	{
 		CServerEntry **ppNewlist;
 		m_NumServerCapacity += 100;
-		ppNewlist = (CServerEntry **)calloc(m_NumServerCapacity, sizeof(CServerEntry *));
+		ppNewlist = (CServerEntry **)calloc(m_NumServerCapacity, sizeof(CServerEntry *)); // NOLINT(bugprone-sizeof-expression)
 		if(m_NumServers > 0)
-			mem_copy(ppNewlist, m_ppServerlist, m_NumServers * sizeof(CServerEntry *));
+			mem_copy(ppNewlist, m_ppServerlist, m_NumServers * sizeof(CServerEntry *)); // NOLINT(bugprone-sizeof-expression)
 		free(m_ppServerlist);
 		m_ppServerlist = ppNewlist;
 	}

src/engine/client/text.cpp

@@ -715,7 +715,7 @@ public:
 		if(FT_New_Memory_Face(m_FTLibrary, pBuf, Size, 0, &FallbackFont.m_FtFace) == 0)
 		{
 			dbg_msg("textrender", "loaded fallback font from '%s'", pFilename);
-			pFont->m_FtFallbackFonts.emplace_back(std::move(FallbackFont));
+			pFont->m_FtFallbackFonts.emplace_back(FallbackFont);
 
 			return true;
 		}

src/engine/graphics.h

@@ -150,7 +150,7 @@ struct GL_SVertexTex3DStream
 typedef void (*WINDOW_RESIZE_FUNC)(void *pUser);
 
 namespace client_data7 {
-struct CDataSprite;
+struct CDataSprite; // NOLINT(bugprone-forward-declaration-namespace)
 }
 
 class IGraphics : public IInterface

src/engine/server/databases/mysql.cpp

@@ -41,11 +41,6 @@ CMysqlConnection::CMysqlConnection(
 
 CMysqlConnection::~CMysqlConnection()
 {
-#if defined(CONF_SQL)
-	m_pStmt.release();
-	m_pPreparedStmt.release();
-	m_pConnection.release();
-#endif
 }
 
 void CMysqlConnection::Print(IConsole *pConsole, const char *Mode)
@@ -104,9 +99,9 @@ IDbConnection::Status CMysqlConnection::Connect()
 
 	try
 	{
-		m_pConnection.release();
-		m_pPreparedStmt.release();
-		m_pResults.release();
+		m_pConnection.reset();
+		m_pPreparedStmt.reset();
+		m_pResults.reset();
 
 		sql::ConnectOptionsMap connection_properties;
 		connection_properties["hostName"] = sql::SQLString(m_aIp);
@@ -304,7 +299,7 @@ void CMysqlConnection::GetString(int Col, char *pBuffer, int BufferSize) const
 int CMysqlConnection::GetBlob(int Col, unsigned char *pBuffer, int BufferSize) const
 {
 #if defined(CONF_SQL)
-	auto Blob = m_pResults->getBlob(Col);
+	auto *Blob = m_pResults->getBlob(Col);
 	Blob->read((char *)pBuffer, BufferSize);
 	int NumRead = Blob->gcount();
 	delete Blob;

src/engine/server/sql_string_helpers.cpp

@@ -73,7 +73,7 @@ void sqlstr::AgoTimeToString(int AgoTime, char *pAgoString, int Size)
 		Seconds = aTimes[i];
 		str_copy(aName, aaNames[i], sizeof(aName));
 
-		Count = floor((float)AgoTime / (float)Seconds);
+		Count = std::floor((float)AgoTime / (float)Seconds);
 		if(Count != 0)
 		{
 			break;
@@ -98,7 +98,7 @@ void sqlstr::AgoTimeToString(int AgoTime, char *pAgoString, int Size)
 		str_copy(aName2, aaNames[i + 1], sizeof(aName2));
 
 		// add second piece if it's greater than 0
-		int Count2 = floor((float)(AgoTime - (Seconds * Count)) / (float)Seconds2);
+		int Count2 = std::floor((float)(AgoTime - (Seconds * Count)) / (float)Seconds2);
 
 		if(Count2 != 0)
 		{

src/game/client/components/ghost.cpp

@@ -59,14 +59,14 @@ void CGhost::GetNetObjCharacter(CNetObj_Character *pChar, const CGhostCharacter
 	pChar->m_Tick = pGhostChar->m_Tick;
 }
 
-CGhost::CGhostPath::CGhostPath(CGhostPath &&Other) :
+CGhost::CGhostPath::CGhostPath(CGhostPath &&Other) noexcept :
 	m_ChunkSize(Other.m_ChunkSize), m_NumItems(Other.m_NumItems), m_lChunks(std::move(Other.m_lChunks))
 {
 	Other.m_NumItems = 0;
 	Other.m_lChunks.clear();
 }
 
-CGhost::CGhostPath &CGhost::CGhostPath::operator=(CGhostPath &&Other)
+CGhost::CGhostPath &CGhost::CGhostPath::operator=(CGhostPath &&Other) noexcept
 {
 	Reset(Other.m_ChunkSize);
 	m_NumItems = Other.m_NumItems;

src/game/client/components/ghost.h

@@ -68,8 +68,8 @@ private:
 		CGhostPath(const CGhostPath &Other) = delete;
 		CGhostPath &operator=(const CGhostPath &Other) = delete;
 
-		CGhostPath(CGhostPath &&Other);
-		CGhostPath &operator=(CGhostPath &&Other);
+		CGhostPath(CGhostPath &&Other) noexcept;
+		CGhostPath &operator=(CGhostPath &&Other) noexcept;
 
 		void Reset(int ChunkSize = 25 * 60); // one minute with default snap rate
 		void SetSize(int Items);

src/game/client/components/maplayers.cpp

@@ -601,7 +601,7 @@ void CMapLayers::OnMapLoad()
 
 						if(!DoTextureCoords)
 						{
-							tmpTiles.reserve((size_t)(pTMap->m_Width * pTMap->m_Height));
+							tmpTiles.reserve((size_t)pTMap->m_Width * pTMap->m_Height);
 							tmpBorderTopTiles.reserve((size_t)pTMap->m_Width);
 							tmpBorderBottomTiles.reserve((size_t)pTMap->m_Width);
 							tmpBorderLeftTiles.reserve((size_t)pTMap->m_Height);
@@ -610,7 +610,7 @@ void CMapLayers::OnMapLoad()
 						}
 						else
 						{
-							tmpTileTexCoords.reserve((size_t)(pTMap->m_Width * pTMap->m_Height));
+							tmpTileTexCoords.reserve((size_t)pTMap->m_Width * pTMap->m_Height);
 							tmpBorderTopTilesTexCoords.reserve((size_t)pTMap->m_Width);
 							tmpBorderBottomTilesTexCoords.reserve((size_t)pTMap->m_Width);
 							tmpBorderLeftTilesTexCoords.reserve((size_t)pTMap->m_Height);

src/game/editor/editor.cpp

@@ -2075,7 +2075,7 @@ void CEditor::DoQuadEnvelopes(const array<CQuad> &lQuads, IGraphics::CTextureHan
 {
 	int Num = lQuads.size();
 	CEnvelope **apEnvelope = new CEnvelope *[Num];
-	mem_zero(apEnvelope, sizeof(CEnvelope *) * Num);
+	mem_zero(apEnvelope, sizeof(CEnvelope *) * Num); // NOLINT(bugprone-sizeof-expression)
 	for(int i = 0; i < Num; i++)
 	{
 		if((m_ShowEnvelopePreview == 1 && lQuads[i].m_PosEnv == m_SelectedEnvelope) || m_ShowEnvelopePreview == 2)

src/game/editor/layer_game.cpp

@@ -39,14 +39,14 @@ void CLayerGame::SetTile(int x, int y, CTile tile)
 		CTile nohook = {TILE_NOHOOK};
 		CLayerTiles::SetTile(x, y, nohook);
 		CTile through_cut = {TILE_THROUGH_CUT};
-		m_pEditor->m_Map.m_pFrontLayer->CLayerTiles::SetTile(x, y, through_cut);
+		m_pEditor->m_Map.m_pFrontLayer->CLayerTiles::SetTile(x, y, through_cut); // NOLINT(bugprone-parent-virtual-call)
 	}
 	else
 	{
 		if(m_pEditor->m_Map.m_pFrontLayer && m_pEditor->m_Map.m_pFrontLayer->GetTile(x, y).m_Index == TILE_THROUGH_CUT)
 		{
 			CTile air = {TILE_AIR};
-			m_pEditor->m_Map.m_pFrontLayer->CLayerTiles::SetTile(x, y, air);
+			m_pEditor->m_Map.m_pFrontLayer->CLayerTiles::SetTile(x, y, air); // NOLINT(bugprone-parent-virtual-call)
 		}
 		if(m_pEditor->m_AllowPlaceUnusedTiles || IsValidGameTile(tile.m_Index))
 		{

src/game/editor/layer_tiles.cpp

@@ -1697,12 +1697,12 @@ void CLayerFront::SetTile(int x, int y, CTile tile)
 	if(tile.m_Index == TILE_THROUGH_CUT)
 	{
 		CTile nohook = {TILE_NOHOOK};
-		m_pEditor->m_Map.m_pGameLayer->CLayerTiles::SetTile(x, y, nohook);
+		m_pEditor->m_Map.m_pGameLayer->CLayerTiles::SetTile(x, y, nohook); // NOLINT(bugprone-parent-virtual-call)
 	}
 	else if(tile.m_Index == TILE_AIR && CLayerTiles::GetTile(x, y).m_Index == TILE_THROUGH_CUT)
 	{
 		CTile air = {TILE_AIR};
-		m_pEditor->m_Map.m_pGameLayer->CLayerTiles::SetTile(x, y, air);
+		m_pEditor->m_Map.m_pGameLayer->CLayerTiles::SetTile(x, y, air); // NOLINT(bugprone-parent-virtual-call)
 	}
 	if(m_pEditor->m_AllowPlaceUnusedTiles || IsValidFrontTile(tile.m_Index))
 	{

src/game/server/entity.h

@@ -26,7 +26,7 @@ private:
 public: \
 	void *operator new(size_t Size, int id); \
 	void operator delete(void *p, int id); \
-	void operator delete(void *p); \
+	void operator delete(void *p); /* NOLINT(misc-new-delete-overloads) */ \
 \
 private:
 
@@ -50,7 +50,7 @@ private:
 		ms_PoolUsed##POOLTYPE[id] = 0; \
 		mem_zero(ms_PoolData##POOLTYPE[id], sizeof(POOLTYPE)); \
 	} \
-	void POOLTYPE::operator delete(void *p) \
+	void POOLTYPE::operator delete(void *p) /* NOLINT(misc-new-delete-overloads) */ \
 	{ \
 		int id = (POOLTYPE *)p - (POOLTYPE *)ms_PoolData##POOLTYPE; \
 		dbg_assert(ms_PoolUsed##POOLTYPE[id], "not used"); \

src/game/server/gamecontext.cpp

@@ -2718,7 +2718,7 @@ void CGameContext::AddVote(const char *pDescription, const char *pCommand)
 		Console()->Print(IConsole::OUTPUT_LEVEL_STANDARD, "server", aBuf);
 		return;
 	}
-	while(*pDescription && *pDescription == ' ')
+	while(*pDescription == ' ')
 		pDescription++;
 	if(str_length(pDescription) >= VOTE_DESC_LENGTH || *pDescription == 0)
 	{

src/game/server/score.h

@@ -4,6 +4,7 @@
 #include <atomic>
 #include <memory>
 #include <string>
+#include <utility>
 #include <vector>
 
 #include <engine/map.h>
@@ -149,7 +150,7 @@ public:
 struct CSqlInitData : ISqlData
 {
 	CSqlInitData(std::shared_ptr<CScoreInitResult> pResult) :
-		m_pResult(pResult)
+		m_pResult(std::move(pResult))
 	{
 	}
 	std::shared_ptr<CScoreInitResult> m_pResult;
@@ -161,7 +162,7 @@ struct CSqlInitData : ISqlData
 struct CSqlPlayerRequest : ISqlData
 {
 	CSqlPlayerRequest(std::shared_ptr<CScorePlayerResult> pResult) :
-		m_pResult(pResult)
+		m_pResult(std::move(pResult))
 	{
 	}
 	std::shared_ptr<CScorePlayerResult> m_pResult;
@@ -177,7 +178,7 @@ struct CSqlPlayerRequest : ISqlData
 struct CSqlRandomMapRequest : ISqlData
 {
 	CSqlRandomMapRequest(std::shared_ptr<CScoreRandomMapResult> pResult) :
-		m_pResult(pResult)
+		m_pResult(std::move(pResult))
 	{
 	}
 	std::shared_ptr<CScoreRandomMapResult> m_pResult;
@@ -191,7 +192,7 @@ struct CSqlRandomMapRequest : ISqlData
 struct CSqlScoreData : ISqlData
 {
 	CSqlScoreData(std::shared_ptr<CScorePlayerResult> pResult) :
-		m_pResult(pResult)
+		m_pResult(std::move(pResult))
 	{
 	}
 	virtual ~CSqlScoreData(){};
@@ -224,7 +225,7 @@ struct CSqlTeamScoreData : ISqlData
 struct CSqlTeamSave : ISqlData
 {
 	CSqlTeamSave(std::shared_ptr<CScoreSaveResult> pResult) :
-		m_pResult(pResult)
+		m_pResult(std::move(pResult))
 	{
 	}
 	virtual ~CSqlTeamSave(){};
@@ -241,7 +242,7 @@ struct CSqlTeamSave : ISqlData
 struct CSqlTeamLoad : ISqlData
 {
 	CSqlTeamLoad(std::shared_ptr<CScoreSaveResult> pResult) :
-		m_pResult(pResult)
+		m_pResult(std::move(pResult))
 	{
 	}
 	virtual ~CSqlTeamLoad(){};

src/game/server/teams.h

@@ -6,6 +6,8 @@
 #include <game/server/gamecontext.h>
 #include <game/teamscore.h>
 
+#include <utility>
+
 class CGameTeams
 {
 	int m_TeamState[MAX_CLIENTS];
@@ -123,7 +125,7 @@ public:
 
 	void SetSaving(int TeamID, std::shared_ptr<CScoreSaveResult> SaveResult)
 	{
-		m_pSaveTeamResult[TeamID] = SaveResult;
+		m_pSaveTeamResult[TeamID] = std::move(SaveResult);
 	}
 
 	bool GetSaving(int TeamID)

src/tools/config_common.h

@@ -7,7 +7,7 @@ struct ListDirectoryContext
 	IStorage *pStorage;
 };
 
-void ProcessItem(const char *pItemName, IStorage *pStorage)
+inline void ProcessItem(const char *pItemName, IStorage *pStorage)
 {
 	char aConfig[2048];
 
@@ -44,7 +44,7 @@ static int ListdirCallback(const char *pItemName, int IsDir, int StorageType, vo
 	return 0;
 }
 
-int main(int argc, const char **argv)
+int main(int argc, const char **argv) // NOLINT(misc-definitions-in-headers)
 {
 	dbg_logger_stdout();
 	IStorage *pStorage = CreateLocalStorage();