DB::loadBlock copy removal & DB backend cleanup

* Remove the copy from db::loadBlock by using a pointer to the destination * cleanup db backend, the child backend doesn't have to set their functions as virtual
minetest · May 17, 2016 · 143401451c457da5079b2970fe260acea45bd85a · 1434014
1 parent decbd39
commit 143401451c457da5079b2970fe260acea45bd85a
diff --git a/src/database-dummy.cpp b/src/database-dummy.cpp
@@ -30,13 +30,16 @@ bool Database_Dummy::saveBlock(const v3s16 &pos, const std::string &data)
 	return true;
 }
 
-std::string Database_Dummy::loadBlock(const v3s16 &pos)
+void Database_Dummy::loadBlock(const v3s16 &pos, std::string *block)
 {
 	s64 i = getBlockAsInteger(pos);
 	std::map<s64, std::string>::iterator it = m_database.find(i);
-	if (it == m_database.end())
-		return "";
-	return it->second;
+	if (it == m_database.end()) {
+		*block = "";
+		return;
+	}
+
+	*block = it->second;
 }
 
 bool Database_Dummy::deleteBlock(const v3s16 &pos)

diff --git a/src/database-dummy.h b/src/database-dummy.h
@@ -28,10 +28,10 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 class Database_Dummy : public Database
 {
 public:
-	virtual bool saveBlock(const v3s16 &pos, const std::string &data);
-	virtual std::string loadBlock(const v3s16 &pos);
-	virtual bool deleteBlock(const v3s16 &pos);
-	virtual void listAllLoadableBlocks(std::vector<v3s16> &dst);
+	bool saveBlock(const v3s16 &pos, const std::string &data);
+	void loadBlock(const v3s16 &pos, std::string *block);
+	bool deleteBlock(const v3s16 &pos);
+	void listAllLoadableBlocks(std::vector<v3s16> &dst);
 
 private:
 	std::map<s64, std::string> m_database;

diff --git a/src/database-leveldb.cpp b/src/database-leveldb.cpp
@@ -65,16 +65,13 @@ bool Database_LevelDB::saveBlock(const v3s16 &pos, const std::string &data)
 	return true;
 }
 
-std::string Database_LevelDB::loadBlock(const v3s16 &pos)
+void Database_LevelDB::loadBlock(const v3s16 &pos, std::string *block)
 {
 	std::string datastr;
 	leveldb::Status status = m_database->Get(leveldb::ReadOptions(),
 		i64tos(getBlockAsInteger(pos)), &datastr);
 
-	if(status.ok())
-		return datastr;
-	else
-		return "";
+	*block = (status.ok()) ? datastr : "";
 }
 
 bool Database_LevelDB::deleteBlock(const v3s16 &pos)

diff --git a/src/database-leveldb.h b/src/database-leveldb.h
@@ -34,10 +34,10 @@ class Database_LevelDB : public Database
 	Database_LevelDB(const std::string &savedir);
 	~Database_LevelDB();
 
-	virtual bool saveBlock(const v3s16 &pos, const std::string &data);
-	virtual std::string loadBlock(const v3s16 &pos);
-	virtual bool deleteBlock(const v3s16 &pos);
-	virtual void listAllLoadableBlocks(std::vector<v3s16> &dst);
+	bool saveBlock(const v3s16 &pos, const std::string &data);
+	void loadBlock(const v3s16 &pos, std::string *block);
+	bool deleteBlock(const v3s16 &pos);
+	void listAllLoadableBlocks(std::vector<v3s16> &dst);
 
 private:
 	leveldb::DB *m_database;

diff --git a/src/database-redis.cpp b/src/database-redis.cpp
@@ -101,7 +101,7 @@ bool Database_Redis::saveBlock(const v3s16 &pos, const std::string &data)
 	return true;
 }
 
-std::string Database_Redis::loadBlock(const v3s16 &pos)
+void Database_Redis::loadBlock(const v3s16 &pos, std::string *block)
 {
 	std::string tmp = i64tos(getBlockAsInteger(pos));
 	redisReply *reply = static_cast<redisReply *>(redisCommand(ctx,
@@ -111,12 +111,13 @@ std::string Database_Redis::loadBlock(const v3s16 &pos)
 		throw FileNotGoodException(std::string(
 			"Redis command 'HGET %s %s' failed: ") + ctx->errstr);
 	}
+
 	switch (reply->type) {
 	case REDIS_REPLY_STRING: {
-		std::string str(reply->str, reply->len);
+		*block = std::string(reply->str, reply->len);
 		// std::string copies the memory so this won't cause any problems
 		freeReplyObject(reply);
-		return str;
+		return;
 	}
 	case REDIS_REPLY_ERROR: {
 		std::string errstr(reply->str, reply->len);
@@ -127,11 +128,13 @@ std::string Database_Redis::loadBlock(const v3s16 &pos)
 			"Redis command 'HGET %s %s' errored: ") + errstr);
 	}
 	case REDIS_REPLY_NIL: {
+		*block = "";
 		// block not found in database
 		freeReplyObject(reply);
-		return "";
+		return;
 	}
 	}
+
 	errorstream << "loadBlock: loading block " << PP(pos)
 		<< " returned invalid reply type " << reply->type
 		<< ": " << std::string(reply->str, reply->len) << std::endl;

diff --git a/src/database-redis.h b/src/database-redis.h
@@ -36,13 +36,13 @@ class Database_Redis : public Database
 	Database_Redis(Settings &conf);
 	~Database_Redis();
 
-	virtual void beginSave();
-	virtual void endSave();
+	void beginSave();
+	void endSave();
 
-	virtual bool saveBlock(const v3s16 &pos, const std::string &data);
-	virtual std::string loadBlock(const v3s16 &pos);
-	virtual bool deleteBlock(const v3s16 &pos);
-	virtual void listAllLoadableBlocks(std::vector<v3s16> &dst);
+	bool saveBlock(const v3s16 &pos, const std::string &data);
+	void loadBlock(const v3s16 &pos, std::string *block);
+	bool deleteBlock(const v3s16 &pos);
+	void listAllLoadableBlocks(std::vector<v3s16> &dst);
 
 private:
 	redisContext *ctx;

diff --git a/src/database-sqlite3.cpp b/src/database-sqlite3.cpp
@@ -237,28 +237,25 @@ bool Database_SQLite3::saveBlock(const v3s16 &pos, const std::string &data)
 	return true;
 }
 
-std::string Database_SQLite3::loadBlock(const v3s16 &pos)
+void Database_SQLite3::loadBlock(const v3s16 &pos, std::string *block)
 {
 	verifyDatabase();
 
 	bindPos(m_stmt_read, pos);
 
 	if (sqlite3_step(m_stmt_read) != SQLITE_ROW) {
 		sqlite3_reset(m_stmt_read);
-		return "";
+		return;
 	}
+
 	const char *data = (const char *) sqlite3_column_blob(m_stmt_read, 0);
 	size_t len = sqlite3_column_bytes(m_stmt_read, 0);
 
-	std::string s;
-	if (data)
-		s = std::string(data, len);
+	*block = (data) ? std::string(data, len) : "";
 
 	sqlite3_step(m_stmt_read);
 	// We should never get more than 1 row, so ok to reset
 	sqlite3_reset(m_stmt_read);
-
-	return s;
 }
 
 void Database_SQLite3::createDatabase()

diff --git a/src/database-sqlite3.h b/src/database-sqlite3.h
@@ -31,16 +31,16 @@ class Database_SQLite3 : public Database
 {
 public:
 	Database_SQLite3(const std::string &savedir);
+	~Database_SQLite3();
 
-	virtual void beginSave();
-	virtual void endSave();
+	void beginSave();
+	void endSave();
 
-	virtual bool saveBlock(const v3s16 &pos, const std::string &data);
-	virtual std::string loadBlock(const v3s16 &pos);
-	virtual bool deleteBlock(const v3s16 &pos);
-	virtual void listAllLoadableBlocks(std::vector<v3s16> &dst);
-	virtual bool initialized() const { return m_initialized; }
-	~Database_SQLite3();
+	bool saveBlock(const v3s16 &pos, const std::string &data);
+	void loadBlock(const v3s16 &pos, std::string *block);
+	bool deleteBlock(const v3s16 &pos);
+	void listAllLoadableBlocks(std::vector<v3s16> &dst);
+	bool initialized() const { return m_initialized; }
 
 private:
 	// Open the database

diff --git a/src/database.h b/src/database.h
@@ -38,7 +38,7 @@ class Database
 	virtual void endSave() {}
 
 	virtual bool saveBlock(const v3s16 &pos, const std::string &data) = 0;
-	virtual std::string loadBlock(const v3s16 &pos) = 0;
+	virtual void loadBlock(const v3s16 &pos, std::string *block) = 0;
 	virtual bool deleteBlock(const v3s16 &pos) = 0;
 
 	static s64 getBlockAsInteger(const v3s16 &pos);

diff --git a/src/main.cpp b/src/main.cpp
@@ -948,7 +948,8 @@ static bool migrate_database(const GameParams &game_params, const Settings &cmd_
 	for (std::vector<v3s16>::const_iterator it = blocks.begin(); it != blocks.end(); ++it) {
 		if (kill) return false;
 
-		const std::string &data = old_db->loadBlock(*it);
+		std::string data;
+		old_db->loadBlock(*it, &data);
 		if (!data.empty()) {
 			new_db->saveBlock(*it, data);
 		} else {

diff --git a/src/map.cpp b/src/map.cpp
@@ -3442,8 +3442,7 @@ MapBlock* ServerMap::loadBlock(v3s16 blockpos)
 	v2s16 p2d(blockpos.X, blockpos.Z);
 
 	std::string ret;
-
-	ret = dbase->loadBlock(blockpos);
+	dbase->loadBlock(blockpos, &ret);
 	if (ret != "") {
 		loadBlock(&ret, blockpos, createSector(p2d), false);
 		return getBlockNoCreateNoEx(blockpos);