diff --git a/ext/lmdb-safe/lmdb-safe.hh b/ext/lmdb-safe/lmdb-safe.hh index 1c62d8802964..1cb7aa41e8d3 100644 --- a/ext/lmdb-safe/lmdb-safe.hh +++ b/ext/lmdb-safe/lmdb-safe.hh @@ -174,54 +174,50 @@ struct MDBOutVal } #ifndef DNSDIST - template ::value, - T>::type* = nullptr> const - T get() - { - T ret; - - size_t offset = LMDBLS::LScheckHeaderAndGetSize(this, sizeof(T)); - - memcpy(&ret, reinterpret_cast(d_mdbval.mv_data)+offset, sizeof(T)); - - static_assert(sizeof(T) == 4, "this code currently only supports 32 bit integers"); - ret = ntohl(ret); - return ret; - } - - template ::value, - T>::type* = nullptr> const - T getNoStripHeader() - { - T ret; - if(d_mdbval.mv_size != sizeof(T)) - throw std::runtime_error("MDB data has wrong length for type"); - - memcpy(&ret, d_mdbval.mv_data, sizeof(T)); - - static_assert(sizeof(T) == 4, "this code currently only supports 32 bit integers"); - ret = ntohl(ret); - return ret; - } + template ::value, T>::type* = nullptr> + T get() const; + template ::value, T>::type* = nullptr> + T getNoStripHeader() const; #endif /* ifndef DNSDIST */ - template ::value,T>::type* = nullptr> + template ::value, T>::type* = nullptr> T get() const; - #ifndef DNSDIST - template ::value,T>::type* = nullptr> + template ::value, T>::type* = nullptr> T getNoStripHeader() const; #endif MDB_val d_mdbval; }; +#ifndef DNSDIST +template <> +inline uint32_t MDBOutVal::get() const +{ + uint32_t ret = 0; + size_t offset = LMDBLS::LScheckHeaderAndGetSize(this, sizeof(uint32_t)); + // NOLINTNEXTLINE + memcpy(&ret, reinterpret_cast(d_mdbval.mv_data) + offset, sizeof(uint32_t)); + ret = ntohl(ret); + return ret; +} + +template <> +inline uint32_t MDBOutVal::getNoStripHeader() const +{ + uint32_t ret = 0; + if (d_mdbval.mv_size != sizeof(uint32_t)) { + throw std::runtime_error("MDB data has wrong length for type"); + } + + memcpy(&ret, d_mdbval.mv_data, sizeof(uint32_t)); + ret = ntohl(ret); + return ret; +} +#endif /* ifndef DNSDIST */ + template<> inline std::string MDBOutVal::get() const { #ifndef DNSDIST diff --git a/ext/lmdb-safe/lmdb-typed.cc b/ext/lmdb-safe/lmdb-typed.cc index e9a2d1e2c089..ff63ea8f2c8f 100644 --- a/ext/lmdb-safe/lmdb-typed.cc +++ b/ext/lmdb-safe/lmdb-typed.cc @@ -1,28 +1,29 @@ #include "lmdb-typed.hh" #include "pdns/dns_random.hh" -unsigned int MDBGetMaxID(MDBRWTransaction& txn, MDBDbi& dbi) +uint32_t MDBGetMaxID(MDBRWTransaction& txn, MDBDbi& dbi) { auto cursor = txn->getRWCursor(dbi); MDBOutVal maxidval{}; MDBOutVal maxcontent{}; - unsigned int maxid{0}; + uint32_t maxid{0}; if (cursor.get(maxidval, maxcontent, MDB_LAST) == 0) { - maxid = maxidval.getNoStripHeader(); + maxid = maxidval.getNoStripHeader(); } return maxid; } -unsigned int MDBGetRandomID(MDBRWTransaction& txn, MDBDbi& dbi) +uint32_t MDBGetRandomID(MDBRWTransaction& txn, MDBDbi& dbi) { auto cursor = txn->getRWCursor(dbi); - unsigned int newID = 0; + uint32_t newID = 0; for (int attempts = 0; attempts < 20; attempts++) { MDBOutVal key{}; MDBOutVal content{}; - // dns_random generates a random number in [0..signed_int_max-1]. We add 1 to avoid 0 and allow type_max. - // 0 is avoided because the put() interface uses it to mean "please allocate a number for me" + // dns_random generates a random number in [0..signed_int_max-1]. We add 1 to avoid 0 + // and allow type_max. 0 is avoided because the put() interface uses it to mean + // "please allocate a number for me". newID = dns_random(std::numeric_limits::max()) + 1; if (cursor.find(MDBInVal(newID), key, content) != 0) { return newID; diff --git a/ext/lmdb-safe/lmdb-typed.hh b/ext/lmdb-safe/lmdb-typed.hh index 14e775e1bcc6..be5608f71827 100644 --- a/ext/lmdb-safe/lmdb-typed.hh +++ b/ext/lmdb-safe/lmdb-typed.hh @@ -1,9 +1,6 @@ #pragma once #include -#include -#include -#include #include #include @@ -13,6 +10,7 @@ #include #include #include +#include #include "lmdb-safe.hh" @@ -31,19 +29,19 @@ /** * LMDB ID Vector Type. */ -typedef std::vector LmdbIdVec; +using LmdbIdVec = std::vector; /** * Return the highest ID used in a database. Returns 0 for an empty DB. This makes us * start everything at ID=1, which might make it possible to treat id 0 as special. */ -unsigned int MDBGetMaxID(MDBRWTransaction& txn, MDBDbi& dbi); +uint32_t MDBGetMaxID(MDBRWTransaction& txn, MDBDbi& dbi); /** * Return a randomly generated ID that is unique and not zero. May throw if the database * is very full. */ -unsigned int MDBGetRandomID(MDBRWTransaction& txn, MDBDbi& dbi); +uint32_t MDBGetRandomID(MDBRWTransaction& txn, MDBDbi& dbi); /** * This is our serialization interface. It can be specialized for other types. @@ -75,7 +73,7 @@ inline std::string keyConv(const T& value); template ::value, T>::type* = nullptr> inline std::string keyConv(const T& value) { - return std::string((char*)&value, sizeof(value)); + return string{(char*)&value, sizeof(value)}; } /** @@ -93,7 +91,7 @@ namespace { throw std::runtime_error("combined key too short to get ID from"); } - MDBOutVal ret; + MDBOutVal ret{}; ret.d_mdbval.mv_data = combined.d_mdbval.mv_data; ret.d_mdbval.mv_size = combined.d_mdbval.mv_size - sizeof(uint32_t); @@ -105,7 +103,7 @@ namespace { throw std::runtime_error("combined key too short to get ID from"); } - MDBOutVal ret; + MDBOutVal ret{}; ret.d_mdbval.mv_data = (char*) combined.d_mdbval.mv_data + combined.d_mdbval.mv_size - sizeof(uint32_t); ret.d_mdbval.mv_size = sizeof(uint32_t); @@ -115,11 +113,11 @@ namespace { inline std::string makeCombinedKey(MDBInVal key, MDBInVal val) { std::string lenprefix(sizeof(uint16_t), '\0'); - std::string skey((char*) key.d_mdbval.mv_data, key.d_mdbval.mv_size); - std::string sval((char*) val.d_mdbval.mv_data, val.d_mdbval.mv_size); + std::string skey(static_cast(key.d_mdbval.mv_data), key.d_mdbval.mv_size); + std::string sval(static_cast(val.d_mdbval.mv_data), val.d_mdbval.mv_size); if (val.d_mdbval.mv_size != 0 && // empty val case, for range queries - val.d_mdbval.mv_size != 4) { // uint32_t case + val.d_mdbval.mv_size != sizeof(uint32_t)) { throw std::runtime_error("got wrong size value in makeCombinedKey"); } @@ -147,25 +145,23 @@ struct LMDBIndexOps { explicit LMDBIndexOps(Parent* parent) : d_parent(parent){} - void put(MDBRWTransaction& txn, const Class& t, uint32_t id, int flags=0) + void put(MDBRWTransaction& txn, const Class& type, uint32_t idVal, int flags = 0) { - std::string sempty(""); - MDBInVal empty(sempty); - - auto scombined = makeCombinedKey(keyConv(d_parent->getMember(t)), id); + auto scombined = makeCombinedKey(keyConv(d_parent->getMember(type)), idVal); MDBInVal combined(scombined); // if the entry existed already, this will just update the timestamp/txid in the LS header. This is intentional, so objects and their indexes always get synced together. - txn->put(d_idx, combined, empty, flags); + txn->put(d_idx, combined, string{}, flags); } - void del(MDBRWTransaction& txn, const Class& t, uint32_t id) + void del(MDBRWTransaction& txn, const Class& type, uint32_t idVal) { - auto scombined = makeCombinedKey(keyConv(d_parent->getMember(t)), id); + auto scombined = makeCombinedKey(keyConv(d_parent->getMember(type)), idVal); MDBInVal combined(scombined); - if(int rc = txn->del(d_idx, combined)) { - throw std::runtime_error("Error deleting from index: " + std::string(mdb_strerror(rc))); + int errCode = txn->del(d_idx, combined); + if (errCode != 0) { + throw std::runtime_error("Error deleting from index: " + std::string(mdb_strerror(errCode))); } } @@ -173,84 +169,86 @@ struct LMDBIndexOps { d_idx = env->openDB(str, flags); } + MDBDbi d_idx; Parent* d_parent; - }; /** This is an index on a field in a struct, it derives from the LMDBIndexOps */ -template +template struct index_on : LMDBIndexOps> { - index_on() : LMDBIndexOps>(this) + index_on() : + LMDBIndexOps>(this) {} - static Type getMember(const Class& c) + static Type getMember(const Class& klass) { - return c.*PtrToMember; + return klass.*PtrToMember; } - typedef Type type; + using type = Type; }; /** This is a calculated index */ -template -struct index_on_function : LMDBIndexOps > +template +struct index_on_function : LMDBIndexOps> { - index_on_function() : LMDBIndexOps >(this) + index_on_function() : + LMDBIndexOps>(this) {} - static Type getMember(const Class& c) + static Type getMember(const Class& klass) { - Func f; - return f(c); + Func function; + return function(klass); } - typedef Type type; + using type = Type; }; /** nop index, so we can fill our N indexes, even if you don't use them all */ struct nullindex_t { - template - void put(MDBRWTransaction& /* txn */, const Class& /* t */, uint32_t /* id */, int /* flags */ =0) + template + void put(MDBRWTransaction& /* txn */, const Class& /* t */, uint32_t /* id */, int /* flags */ = 0) {} - template + template void del(MDBRWTransaction& /* txn */, const Class& /* t */, uint32_t /* id */) {} void openDB(std::shared_ptr& /* env */, string_view /* str */, int /* flags */) { - } - typedef uint32_t type; // dummy + + using type = uint32_t; // dummy }; /** The main class. Templatized only on the indexes and typename right now */ -template +template class TypedDBI { + // we get a lot of our smarts from this tuple, it enables get<0> etc + using tuple_t = std::tuple; + tuple_t d_tuple; + +private: + template + auto openDB(string_view& name) + { + std::get(d_tuple).openDB(d_env, std::string(name) + "_" + std::to_string(N), MDB_CREATE); + } + public: - TypedDBI(std::shared_ptr env, string_view name) - : d_env(std::move(env)), d_name(name) + TypedDBI(std::shared_ptr env, string_view name) : + d_env(std::move(env)), d_name(name) { d_main = d_env->openDB(name, MDB_CREATE); - - // now you might be tempted to go all MPL on this so we can get rid of the - // ugly macro. I'm not very receptive to that idea since it will make things - // EVEN uglier. -#define openMacro(N) std::get(d_tuple).openDB(d_env, std::string(name)+"_"#N, MDB_CREATE); - openMacro(0); - openMacro(1); - openMacro(2); - openMacro(3); -#undef openMacro + openDB<0>(name); + openDB<1>(name); + openDB<2>(name); + openDB<3>(name); } - - // we get a lot of our smarts from this tuple, it enables get<0> etc - typedef std::tuple tuple_t; - tuple_t d_tuple; - // We support readonly and rw transactions. Here we put the Readonly operations // which get sourced by both kinds of transactions template @@ -279,9 +277,10 @@ public: //! Get item with id, from main table directly bool get(uint32_t id, T& t) { - MDBOutVal data; - if((*d_parent.d_txn)->get(d_parent.d_parent->d_main, id, data)) + MDBOutVal data{}; + if((*d_parent.d_txn)->get(d_parent.d_parent->d_main, id, data)) { return false; + } deserializeFromBuffer(data.get(), t); return true; @@ -367,44 +366,46 @@ public: // id.d_mdbval.mv_size -= LS_HEADER_SIZE; // id.d_mdbval.mv_data = (char*)d_id.d_mdbval.mv_data + LS_HEADER_SIZE; - - if(d_on_index) { - if((*d_parent->d_txn)->get(d_parent->d_parent->d_main, d_id, d_data)) + if (d_on_index) { + if ((*d_parent->d_txn)->get(d_parent->d_parent->d_main, d_id, d_data)) { throw std::runtime_error("Missing id in constructor"); + } deserializeFromBuffer(d_data.get(), d_t); } - else + else { deserializeFromBuffer(d_id.get(), d_t); + } } - explicit iter_t(Parent* parent, typename Parent::cursor_t&& cursor, const std::string& prefix) : + explicit iter_t(Parent* parent, typename Parent::cursor_t&& cursor, std::string prefix) : d_parent(parent), d_cursor(std::move(cursor)), d_on_index(true), // is this an iterator on main database or on index? d_one_key(false), - d_prefix(prefix), - d_end(false) + d_prefix(std::move(prefix)) { - if(d_end) + if (d_end) { return; + } - if(d_cursor.get(d_key, d_id, MDB_GET_CURRENT)) { + if (d_cursor.get(d_key, d_id, MDB_GET_CURRENT)) { d_end = true; return; } d_id = getIDFromCombinedKey(d_key); - if(d_on_index) { - if((*d_parent->d_txn)->get(d_parent->d_parent->d_main, d_id, d_data)) + if (d_on_index) { + if ((*d_parent->d_txn)->get(d_parent->d_parent->d_main, d_id, d_data)) { throw std::runtime_error("Missing id in constructor"); + } deserializeFromBuffer(d_data.get(), d_t); } - else + else { deserializeFromBuffer(d_id.get(), d_t); + } } - // std::function filter; void del() { @@ -434,8 +435,8 @@ public: // implements generic ++ or -- iter_t& genoperator(MDB_cursor_op op) { - MDBOutVal data; - int rc; + MDBOutVal data{}; + int rc = 0; // next:; if (!d_one_key) { rc = d_cursor.get(d_key, d_id, op); @@ -443,7 +444,7 @@ public: if(d_one_key || rc == MDB_NOTFOUND) { d_end = true; } - else if(rc) { + else if(rc != 0) { throw std::runtime_error("in genoperator, " + std::string(mdb_strerror(rc))); } else if(!d_prefix.empty() && @@ -461,8 +462,9 @@ public: if(d_on_index) { d_id = getIDFromCombinedKey(d_key); - if((*d_parent->d_txn)->get(d_parent->d_parent->d_main, d_id, data)) + if ((*d_parent->d_txn)->get(d_parent->d_parent->d_main, d_id, data)) { throw std::runtime_error("Missing id field"); + } // if(filter && !filter(data)) // goto next; @@ -490,13 +492,11 @@ public: // get ID this iterator points to uint32_t getID() { - if(d_on_index) { + if (d_on_index) { // return d_id.get(); return d_id.getNoStripHeader(); } - else { - return d_key.getNoStripHeader(); - } + return d_key.getNoStripHeader(); } const MDBOutVal& getKey() @@ -504,13 +504,14 @@ public: return d_key; } - // transaction we are part of Parent* d_parent; typename Parent::cursor_t d_cursor; // gcc complains if I don't zero-init these, which is worrying XXX - MDBOutVal d_key{{0,0}}, d_data{{0,0}}, d_id{{0,0}}; + MDBOutVal d_key{{0, nullptr}}; + MDBOutVal d_data{{0, nullptr}}; + MDBOutVal d_id{{0, nullptr}}; bool d_on_index; bool d_one_key; std::string d_prefix; @@ -523,7 +524,8 @@ public: { typename Parent::cursor_t cursor = (*d_parent.d_txn)->getCursor(std::get(d_parent.d_parent->d_tuple).d_idx); - MDBOutVal out, id; + MDBOutVal out{}; + MDBOutVal id{}; if(cursor.get(out, id, op)) { // on_index, one_key, end @@ -549,7 +551,8 @@ public: { typename Parent::cursor_t cursor = (*d_parent.d_txn)->getCursor(d_parent.d_parent->d_main); - MDBOutVal out, id; + MDBOutVal out{}; + MDBOutVal id{}; if(cursor.get(out, id, MDB_FIRST)) { // on_index, one_key, end @@ -572,7 +575,8 @@ public: std::string keystr = makeCombinedKey(keyConv(key), MDBInVal("")); MDBInVal in(keystr); - MDBOutVal out, id; + MDBOutVal out{}; + MDBOutVal id{}; out.d_mdbval = in.d_mdbval; if(cursor.get(out, id, op)) { @@ -604,7 +608,8 @@ public: std::string keyString=makeCombinedKey(keyConv(key), MDBInVal("")); MDBInVal in(keyString); - MDBOutVal out, id; + MDBOutVal out{}; + MDBOutVal id{}; out.d_mdbval = in.d_mdbval; if(cursor.get(out, id, MDB_SET)) { @@ -623,7 +628,8 @@ public: std::string keyString=makeCombined(keyConv(key), MDBInVal("")); MDBInVal in(keyString); - MDBOutVal out, id; + MDBOutVal out{}; + MDBOutVal id{}; out.d_mdbval = in.d_mdbval; if(cursor.get(out, id, MDB_SET_RANGE) || @@ -643,7 +649,8 @@ public: std::string keyString=makeCombinedKey(keyConv(key), MDBInVal("")); MDBInVal in(keyString); - MDBOutVal out, id; + MDBOutVal out{}; + MDBOutVal id{}; out.d_mdbval = in.d_mdbval; int rc = cursor.get(out, id, MDB_SET_RANGE); @@ -664,7 +671,7 @@ public: if (sthiskey == keyString) { auto _id = getIDFromCombinedKey(out); uint64_t ts = LMDBLS::LSgetTimestamp(id.getNoStripHeader()); - uint32_t __id = _id.getNoStripHeader(); + auto __id = _id.getNoStripHeader(); if (onlyOldest) { if (ts < oldestts) { @@ -715,26 +722,24 @@ public: return d_txn; } - typedef MDBROCursor cursor_t; + using cursor_t = MDBROCursor; TypedDBI* d_parent; std::shared_ptr d_txn; }; - class RWTransaction : public ReadonlyOperations { public: - explicit RWTransaction(TypedDBI* parent) : ReadonlyOperations(*this), d_parent(parent) + explicit RWTransaction(TypedDBI* parent) : + ReadonlyOperations(*this), d_parent(parent), d_txn(std::make_shared(d_parent->d_env->getRWTransaction())) { - d_txn = std::make_shared(d_parent->d_env->getRWTransaction()); } explicit RWTransaction(TypedDBI* parent, std::shared_ptr txn) : ReadonlyOperations(*this), d_parent(parent), d_txn(txn) { } - RWTransaction(RWTransaction&& rhs) : ReadonlyOperations(*this), d_parent(rhs.d_parent), d_txn(std::move(rhs.d_txn)) @@ -772,8 +777,9 @@ public: void modify(uint32_t id, std::function func) { T t; - if(!this->get(id, t)) - throw std::runtime_error("Could not modify id "+std::to_string(id)); + if (!this->get(id, t)) { + throw std::runtime_error("Could not modify id " + std::to_string(id)); + } func(t); del(id); // this is the lazy way. We could test for changed index fields @@ -784,8 +790,9 @@ public: void del(uint32_t id) { T t; - if(!this->get(id, t)) + if (!this->get(id, t)) { return; + } (*d_txn)->del(d_parent->d_main, id); clearIndex(id, t); @@ -796,7 +803,8 @@ public: { auto cursor = (*d_txn)->getRWCursor(d_parent->d_main); bool first = true; - MDBOutVal key, data; + MDBOutVal key{}; + MDBOutVal data{}; while(!cursor.get(key, data, first ? MDB_FIRST : MDB_NEXT)) { first = false; T t; @@ -818,7 +826,7 @@ public: (*d_txn)->abort(); } - typedef MDBRWCursor cursor_t; + using cursor_t = MDBRWCursor; std::shared_ptr getTransactionHandle() { diff --git a/modules/lmdbbackend/lmdbbackend.cc b/modules/lmdbbackend/lmdbbackend.cc index e632c88ed650..36841e619a68 100644 --- a/modules/lmdbbackend/lmdbbackend.cc +++ b/modules/lmdbbackend/lmdbbackend.cc @@ -20,45 +20,38 @@ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. */ -#include "ext/lmdb-safe/lmdb-safe.hh" -#include -#include -#include -#include -#include -#ifdef HAVE_CONFIG_H +#include "lmdbbackend.hh" + #include "config.h" -#endif -#include "pdns/utility.hh" -#include "pdns/dnsbackend.hh" +#include "ext/lmdb-safe/lmdb-safe.hh" +#include "pdns/arguments.hh" +#include "pdns/base32.hh" #include "pdns/dns.hh" +#include "pdns/dnsbackend.hh" #include "pdns/dnspacket.hh" -#include "pdns/base32.hh" #include "pdns/dnssecinfra.hh" -#include "pdns/pdnsexception.hh" #include "pdns/logger.hh" -#include "pdns/version.hh" -#include "pdns/arguments.hh" -#include "pdns/lock.hh" +#include "pdns/pdnsexception.hh" #include "pdns/uuid-utils.hh" -#include #include -#include +#include +#include #include #include +#include #include - -#include +#include +#include +#include +#include +#include +#include +#include #ifdef HAVE_SYSTEMD #include #endif -#include -#include - -#include "lmdbbackend.hh" - #define SCHEMAVERSION 5 // List the class version here. Default is 0 @@ -66,14 +59,14 @@ BOOST_CLASS_VERSION(LMDBBackend::KeyDataDB, 1) BOOST_CLASS_VERSION(DomainInfo, 1) static bool s_first = true; -static int s_shards = 0; +static uint32_t s_shards = 0; static std::mutex s_lmdbStartupLock; std::pair LMDBBackend::getSchemaVersionAndShards(std::string& filename) { // cerr << "getting schema version for path " << filename << endl; - uint32_t schemaversion; + uint32_t schemaversion = 0; MDB_env* tmpEnv = nullptr; @@ -160,7 +153,7 @@ std::pair LMDBBackend::getSchemaVersionAndShards(std::string throw std::runtime_error("pdns.schemaversion had unexpected size"); } - uint32_t shards; + uint32_t shards = 0; key.mv_data = (char*)"shards"; key.mv_size = strlen((char*)key.mv_data); @@ -364,8 +357,6 @@ void copyIndexDBI(MDB_txn* txn, MDB_dbi sdbi, MDB_dbi tdbi) bool LMDBBackend::upgradeToSchemav5(std::string& filename) { - int rc; - auto currentSchemaVersionAndShards = getSchemaVersionAndShards(filename); uint32_t currentSchemaVersion = currentSchemaVersionAndShards.first; uint32_t shards = currentSchemaVersionAndShards.second; @@ -376,23 +367,23 @@ bool LMDBBackend::upgradeToSchemav5(std::string& filename) MDB_env* env = nullptr; - if ((rc = mdb_env_create(&env)) != 0) { + if (mdb_env_create(&env) != 0) { throw std::runtime_error("mdb_env_create failed"); } - if ((rc = mdb_env_set_maxdbs(env, 20)) != 0) { + if (mdb_env_set_maxdbs(env, 20) != 0) { mdb_env_close(env); throw std::runtime_error("mdb_env_set_maxdbs failed"); } - if ((rc = mdb_env_open(env, filename.c_str(), MDB_NOSUBDIR, 0600)) != 0) { + if (mdb_env_open(env, filename.c_str(), MDB_NOSUBDIR, 0600) != 0) { mdb_env_close(env); throw std::runtime_error("mdb_env_open failed"); } MDB_txn* txn = nullptr; - if ((rc = mdb_txn_begin(env, NULL, 0, &txn)) != 0) { + if (mdb_txn_begin(env, nullptr, 0, &txn) != 0) { mdb_env_close(env); throw std::runtime_error("mdb_txn_begin failed"); } @@ -418,31 +409,32 @@ bool LMDBBackend::upgradeToSchemav5(std::string& filename) std::cerr << "migrating shard " << shardfile << std::endl; MDB_env* shenv = nullptr; - if ((rc = mdb_env_create(&shenv)) != 0) { + if (mdb_env_create(&shenv) != 0) { throw std::runtime_error("mdb_env_create failed"); } - if ((rc = mdb_env_set_maxdbs(shenv, 8)) != 0) { + if (mdb_env_set_maxdbs(shenv, 8) != 0) { mdb_env_close(env); throw std::runtime_error("mdb_env_set_maxdbs failed"); } - if ((rc = mdb_env_open(shenv, shardfile.c_str(), MDB_NOSUBDIR, 0600)) != 0) { + if (mdb_env_open(shenv, shardfile.c_str(), MDB_NOSUBDIR, 0600) != 0) { mdb_env_close(env); throw std::runtime_error("mdb_env_open failed"); } MDB_txn* shtxn = nullptr; - if ((rc = mdb_txn_begin(shenv, NULL, 0, &shtxn)) != 0) { + if (mdb_txn_begin(shenv, nullptr, 0, &shtxn) != 0) { mdb_env_close(env); throw std::runtime_error("mdb_txn_begin failed"); } - MDB_dbi shdbi; + MDB_dbi shdbi = 0; - if ((rc = mdb_dbi_open(shtxn, "records", 0, &shdbi)) != 0) { - if (rc == MDB_NOTFOUND) { + const auto dbiOpenRc = mdb_dbi_open(shtxn, "records", 0, &shdbi); + if (dbiOpenRc != 0) { + if (dbiOpenRc == MDB_NOTFOUND) { mdb_txn_abort(shtxn); mdb_env_close(shenv); continue; @@ -452,9 +444,9 @@ bool LMDBBackend::upgradeToSchemav5(std::string& filename) throw std::runtime_error("mdb_dbi_open shard records failed"); } - MDB_dbi shdbi2; + MDB_dbi shdbi2 = 0; - if ((rc = mdb_dbi_open(shtxn, "records_v5", MDB_CREATE, &shdbi2)) != 0) { + if (mdb_dbi_open(shtxn, "records_v5", MDB_CREATE, &shdbi2) != 0) { mdb_dbi_close(shenv, shdbi); mdb_txn_abort(shtxn); mdb_env_close(shenv); @@ -478,8 +470,8 @@ bool LMDBBackend::upgradeToSchemav5(std::string& filename) mdb_env_close(shenv); } - std::array fromtypeddbi; - std::array totypeddbi; + std::array fromtypeddbi{}; + std::array totypeddbi{}; int index = 0; @@ -487,13 +479,16 @@ bool LMDBBackend::upgradeToSchemav5(std::string& filename) std::cerr << "migrating " << dbname << std::endl; std::string tdbname = dbname + "_v5"; - if ((rc = mdb_dbi_open(txn, dbname.c_str(), 0, &fromtypeddbi[index])) != 0) { + // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-constant-array-index) + if (mdb_dbi_open(txn, dbname.c_str(), 0, &fromtypeddbi[index]) != 0) { mdb_txn_abort(txn); mdb_env_close(env); throw std::runtime_error("mdb_dbi_open typeddbi failed"); } - if ((rc = mdb_dbi_open(txn, tdbname.c_str(), MDB_CREATE, &totypeddbi[index])) != 0) { + // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-constant-array-index) + if (mdb_dbi_open(txn, tdbname.c_str(), MDB_CREATE, &totypeddbi[index]) != 0) { + // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-constant-array-index) mdb_dbi_close(env, fromtypeddbi[index]); mdb_txn_abort(txn); mdb_env_close(env); @@ -501,10 +496,13 @@ bool LMDBBackend::upgradeToSchemav5(std::string& filename) } try { + // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-constant-array-index) copyTypedDBI(txn, fromtypeddbi[index], totypeddbi[index]); } catch (std::exception& e) { + // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-constant-array-index) mdb_dbi_close(env, totypeddbi[index]); + // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-constant-array-index) mdb_dbi_close(env, fromtypeddbi[index]); mdb_txn_abort(txn); mdb_env_close(env); @@ -518,8 +516,8 @@ bool LMDBBackend::upgradeToSchemav5(std::string& filename) index++; } - std::array fromindexdbi; - std::array toindexdbi; + std::array fromindexdbi{}; + std::array toindexdbi{}; index = 0; @@ -528,13 +526,16 @@ bool LMDBBackend::upgradeToSchemav5(std::string& filename) std::cerr << "migrating " << dbname << std::endl; std::string tdbname = dbname + "_v5_0"; - if ((rc = mdb_dbi_open(txn, fdbname.c_str(), 0, &fromindexdbi[index])) != 0) { + // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-constant-array-index) + if (mdb_dbi_open(txn, fdbname.c_str(), 0, &fromindexdbi[index]) != 0) { mdb_txn_abort(txn); mdb_env_close(env); throw std::runtime_error("mdb_dbi_open indexdbi failed"); } - if ((rc = mdb_dbi_open(txn, tdbname.c_str(), MDB_CREATE, &toindexdbi[index])) != 0) { + // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-constant-array-index) + if (mdb_dbi_open(txn, tdbname.c_str(), MDB_CREATE, &toindexdbi[index]) != 0) { + // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-constant-array-index) mdb_dbi_close(env, fromindexdbi[index]); mdb_txn_abort(txn); mdb_env_close(env); @@ -542,10 +543,13 @@ bool LMDBBackend::upgradeToSchemav5(std::string& filename) } try { + // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-constant-array-index) copyIndexDBI(txn, fromindexdbi[index], toindexdbi[index]); } catch (std::exception& e) { + // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-constant-array-index) mdb_dbi_close(env, toindexdbi[index]); + // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-constant-array-index) mdb_dbi_close(env, fromindexdbi[index]); mdb_txn_abort(txn); mdb_env_close(env); @@ -559,16 +563,17 @@ bool LMDBBackend::upgradeToSchemav5(std::string& filename) index++; } - MDB_dbi dbi; + MDB_dbi dbi = 0; // finally, migrate the pdns db - if ((rc = mdb_dbi_open(txn, "pdns", 0, &dbi)) != 0) { + if (mdb_dbi_open(txn, "pdns", 0, &dbi) != 0) { mdb_txn_abort(txn); mdb_env_close(env); throw std::runtime_error("mdb_dbi_open pdns failed"); } - MDB_val key, data; + MDB_val key; + MDB_val data; std::string header(LMDBLS::LS_MIN_HEADER_SIZE, '\0'); @@ -578,16 +583,15 @@ bool LMDBBackend::upgradeToSchemav5(std::string& filename) key.mv_data = (char*)keyname.c_str(); key.mv_size = keyname.size(); - if ((rc = mdb_get(txn, dbi, &key, &data))) { + if (mdb_get(txn, dbi, &key, &data) != 0) { throw std::runtime_error("mdb_get pdns.shards failed"); } - uint32_t value; - if (data.mv_size != sizeof(uint32_t)) { throw std::runtime_error("got non-uint32_t key"); } + uint32_t value = 0; memcpy((void*)&value, data.mv_data, sizeof(uint32_t)); value = htonl(value); @@ -595,17 +599,16 @@ bool LMDBBackend::upgradeToSchemav5(std::string& filename) value = htonl(5); } - std::string sdata((char*)data.mv_data, data.mv_size); - + std::string sdata(static_cast(data.mv_data), data.mv_size); + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-cstyle-cast) std::string stdata = header + std::string((char*)&value, sizeof(uint32_t)); - ; MDB_val tdata; tdata.mv_data = (char*)stdata.c_str(); tdata.mv_size = stdata.size(); - if ((rc = mdb_put(txn, dbi, &key, &tdata, 0)) != 0) { + if (mdb_put(txn, dbi, &key, &tdata, 0) != 0) { throw std::runtime_error("mdb_put failed"); } } @@ -616,7 +619,7 @@ bool LMDBBackend::upgradeToSchemav5(std::string& filename) key.mv_data = (char*)keyname.c_str(); key.mv_size = keyname.size(); - if ((rc = mdb_get(txn, dbi, &key, &data))) { + if (mdb_get(txn, dbi, &key, &data) != 0) { throw std::runtime_error("mdb_get pdns.shards failed"); } @@ -629,20 +632,24 @@ bool LMDBBackend::upgradeToSchemav5(std::string& filename) tdata.mv_data = (char*)stdata.c_str(); tdata.mv_size = stdata.size(); - if ((rc = mdb_put(txn, dbi, &key, &tdata, 0)) != 0) { + if (mdb_put(txn, dbi, &key, &tdata, 0) != 0) { throw std::runtime_error("mdb_put failed"); } } for (int i = 0; i < 4; i++) { + // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-constant-array-index) mdb_drop(txn, fromtypeddbi[i], 1); + // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-constant-array-index) mdb_drop(txn, fromindexdbi[i], 1); } cerr << "txn commit=" << mdb_txn_commit(txn) << endl; for (int i = 0; i < 4; i++) { + // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-constant-array-index) mdb_dbi_close(env, totypeddbi[i]); + // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-constant-array-index) mdb_dbi_close(env, toindexdbi[i]); } mdb_env_close(env); @@ -742,32 +749,47 @@ LMDBBackend::LMDBBackend(const std::string& suffix) auto txn = d_tdomains->getEnv()->getRWTransaction(); - MDBOutVal shards; - if (!txn->get(pdnsdbi, "shards", shards)) { + const auto configShardsTemp = atoi(getArg("shards").c_str()); + if (configShardsTemp < 0) { + throw std::runtime_error("a negative shards value is not supported"); + } + if (configShardsTemp == 0) { + throw std::runtime_error("a shards value of 0 is not supported"); + } + const auto configShards = static_cast(configShardsTemp); + + MDBOutVal shards{}; + if (txn->get(pdnsdbi, "shards", shards) == 0) { s_shards = shards.get(); if (mustDo("lightning-stream") && s_shards != 1) { - throw std::runtime_error(std::string("running with Lightning Stream support enabled requires a database with exactly 1 shard")); + throw std::runtime_error("running with Lightning Stream support enabled requires a database with exactly 1 shard"); } - if (s_shards != atoi(getArg("shards").c_str())) { - g_log << Logger::Warning << "Note: configured number of lmdb shards (" << atoi(getArg("shards").c_str()) << ") is different from on-disk (" << s_shards << "). Using on-disk shard number" << endl; + if (s_shards != configShards) { + g_log << Logger::Warning + << "Note: configured number of lmdb shards (" + << atoi(getArg("shards").c_str()) + << ") is different from on-disk (" + << s_shards + << "). Using on-disk shard number" + << endl; } } else { - s_shards = atoi(getArg("shards").c_str()); + s_shards = configShards; txn->put(pdnsdbi, "shards", s_shards); } - MDBOutVal gotuuid; - if (txn->get(pdnsdbi, "uuid", gotuuid)) { + MDBOutVal gotuuid{}; + if (txn->get(pdnsdbi, "uuid", gotuuid) != 0) { const auto uuid = getUniqueID(); const string uuids(uuid.begin(), uuid.end()); txn->put(pdnsdbi, "uuid", uuids); } - MDBOutVal _schemaversion; - if (txn->get(pdnsdbi, "schemaversion", _schemaversion)) { + MDBOutVal _schemaversion{}; + if (txn->get(pdnsdbi, "schemaversion", _schemaversion) != 0) { // our DB is entirely new, so we need to write the schemaversion txn->put(pdnsdbi, "schemaversion", currentSchemaVersion); }