Merge pull request #452 from karliss/sanitizers2

Fix memory and other problems found by address and UB sanitizers.

client/CBitmapHandler.cpp

@@ -25,9 +25,9 @@ namespace BitmapHandler
 
 bool isPCX(const ui8 *header)//check whether file can be PCX according to header
 {
-	int fSize  = read_le_u32(header + 0);
-	int width  = read_le_u32(header + 4);
-	int height = read_le_u32(header + 8);
+	ui32 fSize  = read_le_u32(header + 0);
+	ui32 width  = read_le_u32(header + 4);
+	ui32 height = read_le_u32(header + 8);
 	return fSize == width*height || fSize == width*height*3;
 }
 

client/CMT.cpp

@@ -985,7 +985,6 @@ void dispose()
 	}
 
 	// cleanup, mostly to remove false leaks from analyzer
-	CResourceHandler::clear();
 	if(CCS)
 	{
 		CCS->musich->release();

client/CServerHandler.cpp

@@ -108,7 +108,7 @@ void CServerHandler::resetStateForLobby(const StartInfo::EMode mode, const std::
 	th = make_unique<CStopWatch>();
 	packsForLobbyScreen.clear();
 	c.reset();
-	si.reset(new StartInfo());
+	si = std::make_shared<StartInfo>();
 	playerNames.clear();
 	si->difficulty = 1;
 	si->mode = mode;
@@ -475,11 +475,13 @@ void CServerHandler::endGameplay(bool closeConnection)
 	client->endGame();
 	vstd::clear_pointer(client);
 
-	// Game is ending
-	// Tell the network thread to reach a stable state
-	CSH->sendClientDisconnecting();
-	logNetwork->info("Closed connection.");
-
+	if(closeConnection)
+	{
+		// Game is ending
+		// Tell the network thread to reach a stable state
+		CSH->sendClientDisconnecting();
+		logNetwork->info("Closed connection.");
+	}
 	if(CMM)
 	{
 		GH.curInt = CMM;

client/battle/CBattleAnimations.cpp

@@ -1129,7 +1129,6 @@ bool CEffectAnimation::init()
 	else // Effects targeted at a specific creature/hex.
 	{
 		const CStack * destStack = owner->getCurrentPlayerInterface()->cb->battleGetStackByPos(destTile, false);
-		Rect & tilePos = owner->bfield[destTile]->pos;
 		BattleEffect be;
 		be.effectID = ID;
 		be.animation = animation;
@@ -1141,30 +1140,27 @@ bool CEffectAnimation::init()
 //			if(effect == 1)
 //				be.maxFrame = 3;
 
-		if(x == -1)
+		be.x = x;
+		be.y = y;
+		if(destTile.isValid())
 		{
-			be.x = tilePos.x + tilePos.w/2 - first->width()/2;
-		}
-		else
-		{
-			be.x = x;
-		}
+			Rect & tilePos = owner->bfield[destTile]->pos;
+			if(x == -1)
+				be.x = tilePos.x + tilePos.w/2 - first->width()/2;
+			if(y == -1)
+			{
+				if(alignToBottom)
+					be.y = tilePos.y + tilePos.h - first->height();
+				else
+					be.y = tilePos.y - first->height()/2;
+			}
 
-		if(y == -1)
-		{
-			if(alignToBottom)
-				be.y = tilePos.y + tilePos.h - first->height();
-			else
-				be.y = tilePos.y - first->height()/2;
-		}
-		else
-		{
-			be.y = y;
+			// Correction for 2-hex creatures.
+			if(destStack != nullptr && destStack->doubleWide())
+				be.x += (destStack->side == BattleSide::ATTACKER ? -1 : 1)*tilePos.w/2;
 		}
 
-		// Correction for 2-hex creatures.
-		if(destStack != nullptr && destStack->doubleWide())
-			be.x += (destStack->side == BattleSide::ATTACKER ? -1 : 1)*tilePos.w/2;
+		assert(be.x != -1 && be.y != -1);
 
 		//Indicate if effect should be drawn on top of everything or just on top of the hex
 		be.position = destTile;

launcher/modManager/cmodlistview_moc.cpp

@@ -24,13 +24,13 @@
 
 void CModListView::setupModModel()
 {
-	modModel = new CModListModel();
-	manager = new CModManager(modModel);
+	modModel = new CModListModel(this);
+	manager = vstd::make_unique<CModManager>(modModel);
 }
 
 void CModListView::setupFilterModel()
 {
-	filterModel = new CModFilterModel(modModel);
+	filterModel = new CModFilterModel(modModel, this);
 
 	filterModel->setFilterKeyColumn(-1); // filter across all columns
 	filterModel->setSortCaseSensitivity(Qt::CaseInsensitive); // to make it more user-friendly

launcher/modManager/cmodlistview_moc.h

@@ -29,7 +29,7 @@ class CModListView : public QWidget
 {
 	Q_OBJECT
 
-	CModManager * manager;
+	std::unique_ptr<CModManager> manager;
 	CModListModel * modModel;
 	CModFilterModel * filterModel;
 	CDownloadManager * dlManager;

lib/CHeroHandler.cpp

@@ -718,8 +718,10 @@ void CHeroHandler::loadExperience()
 	expPerLevel.push_back(34140);
 	while (expPerLevel[expPerLevel.size() - 1] > expPerLevel[expPerLevel.size() - 2])
 	{
-		int i = expPerLevel.size() - 1;
-		expPerLevel.push_back (expPerLevel[i] + (expPerLevel[i] - expPerLevel[i-1]) * 1.2);
+		auto i = expPerLevel.size() - 1;
+		auto diff = expPerLevel[i] - expPerLevel[i-1];
+		diff += diff / 5;
+		expPerLevel.push_back (expPerLevel[i] + diff);
 	}
 	expPerLevel.pop_back();//last value is broken
 }

lib/battle/BattleInfo.cpp

@@ -105,15 +105,15 @@ namespace CGH
 //RNG that works like H3 one
 struct RandGen
 {
-	int seed;
+	ui32 seed;
 
-	void srand(int s)
+	void srand(ui32 s)
 	{
 		seed = s;
 	}
 	void srand(int3 pos)
 	{
-		srand(110291 * pos.x + 167801 * pos.y + 81569);
+		srand(110291 * ui32(pos.x) + 167801 * ui32(pos.y) + 81569);
 	}
 	int rand()
 	{

lib/filesystem/Filesystem.cpp

@@ -22,6 +22,7 @@
 #include "../CStopWatch.h"
 
 std::map<std::string, ISimpleResourceLoader*> CResourceHandler::knownLoaders = std::map<std::string, ISimpleResourceLoader*>();
+CResourceHandler CResourceHandler::globalResourceHandler;
 
 CFilesystemGenerator::CFilesystemGenerator(std::string prefix):
 	filesystem(new CFilesystemList()),
@@ -117,11 +118,6 @@ void CFilesystemGenerator::loadJsonMap(const std::string &mountPoint, const Json
 	}
 }
 
-void CResourceHandler::clear()
-{
-	delete knownLoaders["root"];
-}
-
 ISimpleResourceLoader * CResourceHandler::createInitial()
 {
 	//temporary filesystem that will be used to initialize main one.
@@ -174,7 +170,8 @@ void CResourceHandler::initialize()
 	//    |-saves
 	//    |-config
 
-	knownLoaders["root"] = new CFilesystemList();
+	globalResourceHandler.rootLoader = vstd::make_unique<CFilesystemList>();
+	knownLoaders["root"] = globalResourceHandler.rootLoader.get();
 	knownLoaders["saves"] = new CFilesystemLoader("SAVES/", VCMIDirs::get().userSavePath());
 	knownLoaders["config"] = new CFilesystemLoader("CONFIG/", VCMIDirs::get().userConfigPath());
 

lib/filesystem/Filesystem.h

@@ -75,13 +75,6 @@ class DLL_LINKAGE CResourceHandler
 	 */
 	static void initialize();
 
-	/**
-	 * Semi-debug method to track all possible cases of memory leaks
-	 * Used before exiting application
-	 *
-	 */
-	static void clear();
-
 	/**
 	 * Will load all filesystem data from Json data at this path (normally - config/filesystem.json)
 	 * @param fsConfigURI - URI from which data will be loaded
@@ -103,7 +96,12 @@ class DLL_LINKAGE CResourceHandler
 	 */
 	static ISimpleResourceLoader * createFileSystem(const std::string &prefix, const JsonNode & fsConfig);
 
+	~CResourceHandler() = default;
 private:
 	/** Instance of resource loader */
 	static std::map<std::string, ISimpleResourceLoader*> knownLoaders;
+	static CResourceHandler globalResourceHandler;
+
+	CResourceHandler() {};
+	std::unique_ptr<ISimpleResourceLoader> rootLoader;
 };

lib/mapObjects/MiscObjects.cpp

@@ -544,17 +544,15 @@ int CGCreature::getNumberOfStacks(const CGHeroInstance *hero) const
 	else
 		split = 2;
 
-	int a = 1550811371;
-	int b = -935900487;
-	int c = 1943276003;
-	int d = -1120346418;
-
-	int R1 = a * pos.x + b * pos.y + c * pos.z + d;
-	int R2 = R1 / 65536;
-	int R3 = R2 % 32768;
-	if (R3 < 0)
-		R3 += 32767; //is it ever needed if we do modulo calculus?
-	int R4 = R3 % 100 + 1;
+	ui32 a = 1550811371u;
+	ui32 b = 3359066809u;
+	ui32 c = 1943276003u;
+	ui32 d = 3174620878u;
+
+	ui32 R1 = a * ui32(pos.x) + b * ui32(pos.y) + c * ui32(pos.z) + d;
+	ui32 R2 = (R1 >> 16) & 0x7fff;
+
+	int R4 = R2 % 100 + 1;
 
 	if (R4 <= 20)
 		split -= 1;

lib/serializer/Connection.cpp

@@ -58,7 +58,7 @@ void CConnection::init()
 }
 
 CConnection::CConnection(std::string host, ui16 port, std::string Name, std::string UUID)
-	: iser(this), oser(this), io_service(std::make_shared<asio::io_service>()), connectionID(0), name(Name), uuid(UUID)
+	: io_service(std::make_shared<asio::io_service>()), iser(this), oser(this), name(Name), uuid(UUID), connectionID(0)
 {
 	int i;
 	boost::system::error_code error = asio::error::host_not_found;
@@ -111,12 +111,12 @@ CConnection::CConnection(std::string host, ui16 port, std::string Name, std::str
 	throw std::runtime_error("Can't establish connection :(");
 }
 CConnection::CConnection(std::shared_ptr<TSocket> Socket, std::string Name, std::string UUID)
-	: iser(this), oser(this), socket(Socket), io_service(&Socket->get_io_service()), connectionID(0), name(Name), uuid(UUID)
+	: iser(this), oser(this), socket(Socket), name(Name), uuid(UUID), connectionID(0)
 {
 	init();
 }
-CConnection::CConnection(std::shared_ptr<TAcceptor> acceptor, std::shared_ptr<boost::asio::io_service> Io_service, std::string Name, std::string UUID)
-	: iser(this), oser(this), connectionID(0), name(Name), uuid(UUID)
+CConnection::CConnection(std::shared_ptr<TAcceptor> acceptor, std::shared_ptr<boost::asio::io_service> io_service, std::string Name, std::string UUID)
+	: io_service(io_service), iser(this), oser(this), name(Name), uuid(UUID), connectionID(0)
 {
 	boost::system::error_code error = asio::error::host_not_found;
 	socket = std::make_shared<tcp::socket>(*io_service);

lib/serializer/Connection.h

@@ -49,13 +49,13 @@ typedef boost::asio::basic_socket_acceptor<boost::asio::ip::tcp, boost::asio::so
 class DLL_LINKAGE CConnection
 	: public IBinaryReader, public IBinaryWriter, public std::enable_shared_from_this<CConnection>
 {
-	CConnection();
-
 	void init();
 	void reportState(vstd::CLoggerBase * out) override;
 
 	int write(const void * data, unsigned size) override;
 	int read(void * data, unsigned size) override;
+
+	std::shared_ptr<boost::asio::io_service> io_service; //can be empty if connection made from socket
 public:
 	BinaryDeserializer iser;
 	BinarySerializer oser;
@@ -66,7 +66,6 @@ class DLL_LINKAGE CConnection
 	bool connected;
 	bool myEndianess, contactEndianess; //true if little endian, if endianness is different we'll have to revert received multi-byte vars
 	std::string contactUuid;
-	std::shared_ptr<boost::asio::io_service> io_service;
 	std::string name; //who uses this connection
 	std::string uuid;