Use unique_ptrs for MapSector::m_blocks

src/map.cpp

@@ -116,22 +116,22 @@ MapSector * Map::getSectorNoGenerateNoLock(v2s16 p)
 	return sector;
 }
 
-MapSector * Map::getSectorNoGenerate(v2s16 p)
+MapSector *Map::getSectorNoGenerate(v2s16 p)
 {
 	return getSectorNoGenerateNoLock(p);
 }
 
-MapBlock * Map::getBlockNoCreateNoEx(v3s16 p3d)
+MapBlock *Map::getBlockNoCreateNoEx(v3s16 p3d)
 {
 	v2s16 p2d(p3d.X, p3d.Z);
-	MapSector * sector = getSectorNoGenerate(p2d);
-	if(sector == NULL)
-		return NULL;
+	MapSector *sector = getSectorNoGenerate(p2d);
+	if (!sector)
+		return nullptr;
 	MapBlock *block = sector->getBlockNoCreateNoEx(p3d.Y);
 	return block;
 }
 
-MapBlock * Map::getBlockNoCreate(v3s16 p3d)
+MapBlock *Map::getBlockNoCreate(v3s16 p3d)
 {
 	MapBlock *block = getBlockNoCreateNoEx(p3d);
 	if(block == NULL)
@@ -1795,21 +1795,20 @@ void ServerMap::loadBlock(std::string *blob, v3s16 p3d, MapSector *sector, bool
 			throw SerializationError("ServerMap::loadBlock(): Failed"
 					" to read MapBlock version");
 
-		MapBlock *block = NULL;
-		bool created_new = false;
+		MapBlock *block = nullptr;
+		std::unique_ptr<MapBlock> block_created_new;
 		block = sector->getBlockNoCreateNoEx(p3d.Y);
-		if(block == NULL)
-		{
-			block = sector->createBlankBlockNoInsert(p3d.Y);
-			created_new = true;
+		if (!block) {
+			block_created_new = sector->createBlankBlockNoInsert(p3d.Y);
+			block = block_created_new.get();
 		}
 
 		// Read basic data
 		block->deSerialize(is, version, true);
 
 		// If it's a new block, insert it to the map
-		if (created_new) {
-			sector->insertBlock(block);
+		if (block_created_new) {
+			sector->insertBlock(std::move(block_created_new));
 			ReflowScan scanner(this, m_emerge->ndef);
 			scanner.scan(block, &m_transforming_liquid);
 		}
@@ -1892,8 +1891,7 @@ bool ServerMap::deleteBlock(v3s16 blockpos)
 			return false;
 		// It may not be safe to delete the block from memory at the moment
 		// (pointers to it could still be in use)
-		sector->detachBlock(block);
-		m_detached_blocks.push_back(block);
+		m_detached_blocks.push_back(sector->detachBlock(block));
 	}
 
 	return true;
@@ -1901,10 +1899,11 @@ bool ServerMap::deleteBlock(v3s16 blockpos)
 
 void ServerMap::deleteDetachedBlocks()
 {
-	for (MapBlock *block : m_detached_blocks) {
+	for (const auto &block : m_detached_blocks) {
 		assert(block->isOrphan());
-		delete block;
+		(void)block; // silence unused-variable warning in release builds
 	}
+
 	m_detached_blocks.clear();
 }
 

src/map.h

@@ -465,7 +465,7 @@ private:
 	std::set<v3s16> m_chunks_in_progress;
 
 	// used by deleteBlock() and deleteDetachedBlocks()
-	MapBlockVect m_detached_blocks;
+	std::vector<std::unique_ptr<MapBlock>> m_detached_blocks;
 
 	// Queued transforming water nodes
 	UniqueQueue<v3s16> m_transforming_liquid;

src/mapsector.cpp

@@ -39,16 +39,11 @@ void MapSector::deleteBlocks()
 	// Clear cache
 	m_block_cache = nullptr;
 
-	// Delete all
-	for (auto &block : m_blocks) {
-		delete block.second;
-	}
-
-	// Clear container
+	// Delete all blocks
 	m_blocks.clear();
 }
 
-MapBlock * MapSector::getBlockBuffered(s16 y)
+MapBlock *MapSector::getBlockBuffered(s16 y)
 {
 	MapBlock *block;
 
@@ -57,8 +52,8 @@ MapBlock * MapSector::getBlockBuffered(s16 y)
 	}
 
 	// If block doesn't exist, return NULL
-	std::unordered_map<s16, MapBlock*>::const_iterator n = m_blocks.find(y);
-	block = (n != m_blocks.end() ? n->second : nullptr);
+	auto it = m_blocks.find(y);
+	block = it != m_blocks.end() ? it->second.get() : nullptr;
 
 	// Cache the last result
 	m_block_cache_y = y;
@@ -67,32 +62,31 @@ MapBlock * MapSector::getBlockBuffered(s16 y)
 	return block;
 }
 
-MapBlock * MapSector::getBlockNoCreateNoEx(s16 y)
+MapBlock *MapSector::getBlockNoCreateNoEx(s16 y)
 {
 	return getBlockBuffered(y);
 }
 
-MapBlock * MapSector::createBlankBlockNoInsert(s16 y)
+std::unique_ptr<MapBlock> MapSector::createBlankBlockNoInsert(s16 y)
 {
-	assert(getBlockBuffered(y) == NULL);	// Pre-condition
+	assert(getBlockBuffered(y) == nullptr); // Pre-condition
 
 	v3s16 blockpos_map(m_pos.X, y, m_pos.Y);
 
-	MapBlock *block = new MapBlock(m_parent, blockpos_map, m_gamedef);
-
-	return block;
+	return std::make_unique<MapBlock>(m_parent, blockpos_map, m_gamedef);
 }
 
-MapBlock * MapSector::createBlankBlock(s16 y)
+MapBlock *MapSector::createBlankBlock(s16 y)
 {
-	MapBlock *block = createBlankBlockNoInsert(y);
+	std::unique_ptr<MapBlock> block_u = createBlankBlockNoInsert(y);
+	MapBlock *block = block_u.get();
 
-	m_blocks[y] = block;
+	m_blocks[y] = std::move(block_u);
 
 	return block;
 }
 
-void MapSector::insertBlock(MapBlock *block)
+void MapSector::insertBlock(std::unique_ptr<MapBlock> block)
 {
 	s16 block_y = block->getPos().Y;
 
@@ -105,16 +99,16 @@ void MapSector::insertBlock(MapBlock *block)
 	assert(p2d == m_pos);
 
 	// Insert into container
-	m_blocks[block_y] = block;
+	m_blocks[block_y] = std::move(block);
 }
 
 void MapSector::deleteBlock(MapBlock *block)
 {
 	detachBlock(block);
-	delete block;
+	// returned smart-ptr is dropped
 }
 
-void MapSector::detachBlock(MapBlock *block)
+std::unique_ptr<MapBlock> MapSector::detachBlock(MapBlock *block)
 {
 	s16 block_y = block->getPos().Y;
 
@@ -122,16 +116,22 @@ void MapSector::detachBlock(MapBlock *block)
 	m_block_cache = nullptr;
 
 	// Remove from container
-	m_blocks.erase(block_y);
+	auto it = m_blocks.find(block_y);
+	assert(it != m_blocks.end());
+	std::unique_ptr<MapBlock> ret = std::move(it->second);
+	assert(ret.get() == block);
+	m_blocks.erase(it);
 
 	// Mark as removed
 	block->makeOrphan();
+
+	return ret;
 }
 
 void MapSector::getBlocks(MapBlockVect &dest)
 {
 	dest.reserve(dest.size() + m_blocks.size());
 	for (auto &block : m_blocks) {
-		dest.push_back(block.second);
+		dest.push_back(block.second.get());
 	}
 }

src/mapsector.h

@@ -50,16 +50,17 @@ public:
 		return m_pos;
 	}
 
-	MapBlock * getBlockNoCreateNoEx(s16 y);
-	MapBlock * createBlankBlockNoInsert(s16 y);
-	MapBlock * createBlankBlock(s16 y);
+	MapBlock *getBlockNoCreateNoEx(s16 y);
+	std::unique_ptr<MapBlock> createBlankBlockNoInsert(s16 y);
+	MapBlock *createBlankBlock(s16 y);
 
-	void insertBlock(MapBlock *block);
+	void insertBlock(std::unique_ptr<MapBlock> block);
 
 	void deleteBlock(MapBlock *block);
 
 	// Remove a block from the map and the sector without deleting it
-	void detachBlock(MapBlock *block);
+	// Returns an owning ptr to block.
+	std::unique_ptr<MapBlock> detachBlock(MapBlock *block);
 
 	void getBlocks(MapBlockVect &dest);
 
@@ -69,7 +70,7 @@ public:
 protected:
 
 	// The pile of MapBlocks
-	std::unordered_map<s16, MapBlock*> m_blocks;
+	std::unordered_map<s16, std::unique_ptr<MapBlock>> m_blocks;
 
 	Map *m_parent;
 	// Position on parent (in MapBlock widths)