Fix some clang-tidy issues and UB

src/engine/client/serverbrowser_http.cpp

@@ -54,8 +54,8 @@ private:
 		CChooseMaster *m_pParent;
 		CLock m_Lock;
 		std::shared_ptr<CData> m_pData;
-		std::shared_ptr<CHttpRequest> m_pHead PT_GUARDED_BY(m_Lock);
-		std::shared_ptr<CHttpRequest> m_pGet PT_GUARDED_BY(m_Lock);
+		std::shared_ptr<CHttpRequest> m_pHead;
+		std::shared_ptr<CHttpRequest> m_pGet;
 		void Run() override REQUIRES(!m_Lock);
 
 	public:
@@ -174,7 +174,7 @@ void CChooseMaster::CJob::Run()
 	{
 		aTimeMs[i] = -1;
 		const char *pUrl = m_pData->m_aaUrls[aRandomized[i]];
-		std::shared_ptr<CHttpRequest> pHead = std::move(HttpHead(pUrl));
+		std::shared_ptr<CHttpRequest> pHead = HttpHead(pUrl);
 		pHead->Timeout(Timeout);
 		pHead->LogProgress(HTTPLOG::FAILURE);
 		{
@@ -195,7 +195,7 @@ void CChooseMaster::CJob::Run()
 		}
 
 		auto StartTime = time_get_nanoseconds();
-		std::shared_ptr<CHttpRequest> pGet = std::move(HttpGet(pUrl));
+		std::shared_ptr<CHttpRequest> pGet = HttpGet(pUrl);
 		pGet->Timeout(Timeout);
 		pGet->LogProgress(HTTPLOG::FAILURE);
 		{
@@ -296,7 +296,6 @@ private:
 	static bool Validate(json_value *pJson);
 	static bool Parse(json_value *pJson, std::vector<CServerInfo> *pvServers, std::vector<NETADDR> *pvLegacyServers);
 
-	IEngine *m_pEngine;
 	IConsole *m_pConsole;
 	IHttp *m_pHttp;
 
@@ -309,7 +308,6 @@ private:
 };
 
 CServerBrowserHttp::CServerBrowserHttp(IEngine *pEngine, IConsole *pConsole, IHttp *pHttp, const char **ppUrls, int NumUrls, int PreviousBestIndex) :
-	m_pEngine(pEngine),
 	m_pConsole(pConsole),
 	m_pHttp(pHttp),
 	m_pChooseMaster(new CChooseMaster(pEngine, pHttp, Validate, ppUrls, NumUrls, PreviousBestIndex))

src/engine/client/updater.cpp

@@ -163,7 +163,8 @@ bool CUpdater::MoveFile(const char *pFile)
 
 void CUpdater::Update()
 {
-	switch(m_State)
+	auto State = GetCurrentState();
+	switch(State)
 	{
 	case IUpdater::GOT_MANIFEST:
 		PerformUpdate();
@@ -270,6 +271,7 @@ void CUpdater::ParseUpdate()
 				break;
 		}
 	}
+	json_value_free(pVersions);
 }
 
 void CUpdater::InitiateUpdate()
@@ -375,9 +377,9 @@ void CUpdater::CommitUpdate()
 	if(m_ServerUpdate)
 		Success &= ReplaceServer();
 	if(!Success)
-		m_State = FAIL;
+		SetCurrentState(IUpdater::FAIL);
 	else if(m_pClient->State() == IClient::STATE_ONLINE || m_pClient->EditorHasUnsavedData())
-		m_State = NEED_RESTART;
+		SetCurrentState(IUpdater::NEED_RESTART);
 	else
 	{
 		m_pClient->Restart();

src/engine/client/updater.h

@@ -48,7 +48,7 @@ class CUpdater : public IUpdater
 
 	CLock m_Lock;
 
-	int m_State;
+	int m_State GUARDED_BY(m_Lock);
 	char m_aStatus[256] GUARDED_BY(m_Lock);
 	int m_Percent GUARDED_BY(m_Lock);
 	char m_aClientExecTmp[64];
@@ -62,13 +62,13 @@ class CUpdater : public IUpdater
 	bool m_ServerUpdate;
 
 	void AddFileJob(const char *pFile, bool Job);
-	void FetchFile(const char *pFile, const char *pDestPath = nullptr);
+	void FetchFile(const char *pFile, const char *pDestPath = nullptr) REQUIRES(!m_Lock);
 	bool MoveFile(const char *pFile);
 
-	void ParseUpdate();
-	void PerformUpdate();
-	void RunningUpdate();
-	void CommitUpdate();
+	void ParseUpdate() REQUIRES(!m_Lock);
+	void PerformUpdate() REQUIRES(!m_Lock);
+	void RunningUpdate() REQUIRES(!m_Lock);
+	void CommitUpdate() REQUIRES(!m_Lock);
 
 	bool ReplaceClient();
 	bool ReplaceServer();
@@ -82,9 +82,9 @@ public:
 	void GetCurrentFile(char *pBuf, int BufSize) override REQUIRES(!m_Lock);
 	int GetCurrentPercent() override REQUIRES(!m_Lock);
 
-	void InitiateUpdate() override;
+	void InitiateUpdate() REQUIRES(!m_Lock) override;
 	void Init(CHttp *pHttp);
-	void Update() override;
+	void Update() REQUIRES(!m_Lock) override;
 };
 
 #endif

src/engine/shared/http.cpp

@@ -571,7 +571,7 @@ void CHttp::Run(std::shared_ptr<IHttpRequest> pRequest)
 {
 	std::unique_lock Lock(m_Lock);
 	m_Cv.wait(Lock, [this]() { return m_State != UNINITIALIZED; });
-	m_PendingRequests.emplace_back(std::move(std::static_pointer_cast<CHttpRequest>(pRequest)));
+	m_PendingRequests.emplace_back(std::static_pointer_cast<CHttpRequest>(pRequest));
 	curl_multi_wakeup(m_pMultiH);
 }
 
@@ -584,3 +584,12 @@ void CHttp::Shutdown()
 	m_Shutdown = true;
 	curl_multi_wakeup(m_pMultiH);
 }
+
+CHttp::~CHttp()
+{
+	if(!m_pThread)
+		return;
+
+	Shutdown();
+	thread_wait(m_pThread);
+}

src/engine/shared/http.h

@@ -132,7 +132,7 @@ protected:
 
 public:
 	CHttpRequest(const char *pUrl);
-	~CHttpRequest();
+	virtual ~CHttpRequest();
 
 	void Timeout(CTimeout Timeout) { m_Timeout = Timeout; }
 	void MaxResponseSize(int64_t MaxResponseSize) { m_MaxResponseSize = MaxResponseSize; }
@@ -273,6 +273,7 @@ public:
 	// User
 	virtual void Run(std::shared_ptr<IHttpRequest> pRequest) override;
 	void Shutdown() override;
+	~CHttp();
 };
 
 #endif // ENGINE_SHARED_HTTP_H

src/game/client/components/menus.h

@@ -536,6 +536,7 @@ protected:
 
 	public:
 		CCommunityIconLoadJob(CMenus *pMenus, const char *pCommunityId, int StorageType);
+		~CCommunityIconLoadJob();
 
 		CImageInfo &&ImageInfo() { return std::move(m_ImageInfo); }
 	};

src/game/client/components/menus_browser.cpp

@@ -1822,6 +1822,12 @@ CMenus::CCommunityIconLoadJob::CCommunityIconLoadJob(CMenus *pMenus, const char
 {
 }
 
+CMenus::CCommunityIconLoadJob::~CCommunityIconLoadJob()
+{
+	free(m_ImageInfo.m_pData);
+	m_ImageInfo.m_pData = nullptr;
+}
+
 int CMenus::CommunityIconScan(const char *pName, int IsDir, int DirType, void *pUser)
 {
 	const char *pExtension = ".png";