From 143401451c457da5079b2970fe260acea45bd85a Mon Sep 17 00:00:00 2001 From: Loic Blot Date: Sat, 14 May 2016 12:23:15 +0200 Subject: [PATCH] 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 --- src/database-dummy.cpp | 11 +++++++---- src/database-dummy.h | 8 ++++---- src/database-leveldb.cpp | 7 ++----- src/database-leveldb.h | 8 ++++---- src/database-redis.cpp | 11 +++++++---- src/database-redis.h | 12 ++++++------ src/database-sqlite3.cpp | 11 ++++------- src/database-sqlite3.h | 16 ++++++++-------- src/database.h | 2 +- src/main.cpp | 3 ++- src/map.cpp | 3 +-- 11 files changed, 46 insertions(+), 46 deletions(-) diff --git a/src/database-dummy.cpp b/src/database-dummy.cpp index b38db1fb9..ef2148f70 100644 --- 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::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 index 0cf56928e..72100edd0 100644 --- 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 &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 &dst); private: std::map m_database; diff --git a/src/database-leveldb.cpp b/src/database-leveldb.cpp index acd0fd1eb..f46f82b98 100644 --- 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 index 4afe2fdc7..3993db0c3 100644 --- a/src/database-leveldb.h +++ b/src/database-leveldb.h @@ -34,10 +34,10 @@ public: 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 &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 &dst); private: leveldb::DB *m_database; diff --git a/src/database-redis.cpp b/src/database-redis.cpp index 498e9d39a..01427b6f9 100644 --- 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(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 index 45e702c83..3addaa20a 100644 --- a/src/database-redis.h +++ b/src/database-redis.h @@ -36,13 +36,13 @@ public: 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 &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 &dst); private: redisContext *ctx; diff --git a/src/database-sqlite3.cpp b/src/database-sqlite3.cpp index 56f937bf2..2a23e2ea0 100644 --- a/src/database-sqlite3.cpp +++ b/src/database-sqlite3.cpp @@ -237,7 +237,7 @@ 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(); @@ -245,20 +245,17 @@ std::string Database_SQLite3::loadBlock(const v3s16 &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 index 04a1825d9..debbc9d8b 100644 --- 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 &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 &dst); + bool initialized() const { return m_initialized; } private: // Open the database diff --git a/src/database.h b/src/database.h index cee7b6fd9..0cf75232f 100644 --- a/src/database.h +++ b/src/database.h @@ -38,7 +38,7 @@ public: 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 index 1b95a9f1c..f63b6a986 100644 --- 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::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 index 66fabaf87..848d2ef13 100644 --- 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); -- 2.25.1