DB::loadBlock copy removal & DB backend cleanup 4125/head
authorLoic Blot <loic.blot@unix-experience.fr>
Sat, 14 May 2016 10:23:15 +0000 (12:23 +0200)
committerLoic Blot <loic.blot@unix-experience.fr>
Tue, 17 May 2016 04:52:16 +0000 (06:52 +0200)
* 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
src/database-dummy.h
src/database-leveldb.cpp
src/database-leveldb.h
src/database-redis.cpp
src/database-redis.h
src/database-sqlite3.cpp
src/database-sqlite3.h
src/database.h
src/main.cpp
src/map.cpp

index b38db1fb966f3c1dcbc5eddd3132b87bed4b2d45..ef2148f70b742ea52db2c6190bdb11eb53820079 100644 (file)
@@ -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)
index 0cf56928e08d314f2ad11edf5fcc172b106cbc0f..72100edd0bdcb1f3c952db1db3a16d6317ab167a 100644 (file)
@@ -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;
index acd0fd1eb7dd3d13edfd4e6b7f845d97f1fbc7b8..f46f82b985937ce24cee3101098e672793db33f6 100644 (file)
@@ -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)
index 4afe2fdc727bc3261803a2b971a788c63b18bcc5..3993db0c3ae7559b02b86ba6d7c7fd62bcb105bd 100644 (file)
@@ -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<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;
index 498e9d39a50169dfa28c864890635bd512263eab..01427b6f96e7ad7e8959880b71790103b526eb3e 100644 (file)
@@ -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;
index 45e702c83463a29c9e0a0e3e84247137ff84f2ae..3addaa20a7ad0c8e6e01fd997f03973d28353dc5 100644 (file)
@@ -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<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;
index 56f937bf21b20bb714c2382d9f06b3121099f1c0..2a23e2ea0ce7e8cf9fea4cbe1c7decbea6d75f53 100644 (file)
@@ -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()
index 04a1825d98368cb65ff0bf60b71b8e46353ee259..debbc9d8bfb6d5287cb5ab0812abd8b2e4bd2b36 100644 (file)
@@ -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
index cee7b6fd96d5589869ee2a5a9aa322193521923b..0cf75232ff0027948787a1ff85e1d5750e523e14 100644 (file)
@@ -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);
index 1b95a9f1c7cf606ec87f16e9fe01b908cf1d4551..f63b6a986e5de3c1f3b11e9634d9dbc4949884f0 100644 (file)
@@ -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 {
index 66fabaf873d234c8b288542adb590bf9cd6fd022..848d2ef13255a715dd0f9c9123431c052704b12c 100644 (file)
@@ -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);