From 4d0dd5e877710c8be02dc568594a94d294e47ca6 Mon Sep 17 00:00:00 2001 From: Matej Matuska <75834032+matejmatuska@users.noreply.github.com> Date: Tue, 3 Sep 2024 14:07:30 +0200 Subject: [PATCH 1/6] Check return values when connecting/creating a database This fixes a bug where when establishing a connection to the src database fails, the execution continues on creating the dst database and returning a 0 exit code. --- convert.cc | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/convert.cc b/convert.cc index 7ee9677..5b8db98 100755 --- a/convert.cc +++ b/convert.cc @@ -54,18 +54,29 @@ int main(int argc, char* argv[]){ DB_ * x = new Libdb(); - x->connect_database(src); + if (!x->connect_database(src)) { + // error is printed in the connect_database function + delete x; + return 1; + } DB_ *b; if (lmdb) b = new LMDB_(); else b = new GDBM_(); - b->create_database(dst); + if (!b->create_database(dst)) { + std::cerr<<"Failed to create destination database\n"; + delete x; + delete b; + return 1; + } if (!b->fill_database(x)){ std::cerr<<"database filling failed\n"; + delete x; + delete b; return 1; } - b->close_db(); + b->close_db(); delete x; delete b; return 0; From bb5989ae26e20f173e3bc2a9ce9c1e6f600af297 Mon Sep 17 00:00:00 2001 From: Matej Matuska <75834032+matejmatuska@users.noreply.github.com> Date: Tue, 3 Sep 2024 14:05:57 +0200 Subject: [PATCH 2/6] Properly initialize the terminating option This removes compilation warnings about uninitialized values. --- convert.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/convert.cc b/convert.cc index 5b8db98..972564d 100755 --- a/convert.cc +++ b/convert.cc @@ -25,7 +25,7 @@ int main(int argc, char* argv[]){ {"src", required_argument, NULL, 's'}, {"lmdb", no_argument, NULL, 'l'}, {"help", no_argument, NULL, 'h'}, - {0} + {0, 0, 0, 0} }; int option_index = 0; while (1) { From 6694d4f4c0dcdfd169766e533dc17f92e0a38c24 Mon Sep 17 00:00:00 2001 From: Matej Matuska <75834032+matejmatuska@users.noreply.github.com> Date: Tue, 3 Sep 2024 14:11:43 +0200 Subject: [PATCH 3/6] Add a virtual destructor to DB_ This removes warning about deleting an object with a non-virtual destructor in the base class. --- convert_db.h | 1 + db.cc | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/convert_db.h b/convert_db.h index 7bc8dfa..2299f41 100644 --- a/convert_db.h +++ b/convert_db.h @@ -39,6 +39,7 @@ class DB_ { virtual bool fill_database(DB_ * old_database); virtual void close_db(); virtual DBC * get_database(); + virtual ~DB_(); }; /* * Libdb class needs only open and read data from libdb database diff --git a/db.cc b/db.cc index 932d517..b0941c7 100644 --- a/db.cc +++ b/db.cc @@ -21,6 +21,10 @@ DBC * DB_::get_database() { return NULL; } void DB_::close_db(){} + +DB_::~DB_() { +} + bool Libdb::connect_database(std::string path){ DB * db; int status; From 6aec16a7d673889e93e737d1c127797f74d754b1 Mon Sep 17 00:00:00 2001 From: Matej Matuska <75834032+matejmatuska@users.noreply.github.com> Date: Wed, 4 Sep 2024 13:10:19 +0200 Subject: [PATCH 4/6] Properly close databases Also implemented LibDB::close_db() which closes both the cursor and db. --- convert.cc | 8 ++++++-- convert_db.h | 3 ++- db.cc | 5 +++++ 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/convert.cc b/convert.cc index 972564d..63377fe 100755 --- a/convert.cc +++ b/convert.cc @@ -56,8 +56,8 @@ int main(int argc, char* argv[]){ DB_ * x = new Libdb(); if (!x->connect_database(src)) { // error is printed in the connect_database function - delete x; - return 1; + delete x; + return 1; } DB_ *b; if (lmdb) @@ -67,15 +67,19 @@ int main(int argc, char* argv[]){ if (!b->create_database(dst)) { std::cerr<<"Failed to create destination database\n"; delete x; + x->close_db(); delete b; return 1; } if (!b->fill_database(x)){ std::cerr<<"database filling failed\n"; + x->close_db(); + b->close_db(); delete x; delete b; return 1; } + x->close_db(); b->close_db(); delete x; delete b; diff --git a/convert_db.h b/convert_db.h index 2299f41..e11b8d3 100644 --- a/convert_db.h +++ b/convert_db.h @@ -39,7 +39,7 @@ class DB_ { virtual bool fill_database(DB_ * old_database); virtual void close_db(); virtual DBC * get_database(); - virtual ~DB_(); + virtual ~DB_(); }; /* * Libdb class needs only open and read data from libdb database @@ -54,6 +54,7 @@ class Libdb: public DB_ { Libdb(); bool connect_database(std::string path); DBC * get_database(); + void close_db(); }; /* * GDBM class provides API for GDBM, allowes to open and create and fill gdbm database diff --git a/db.cc b/db.cc index b0941c7..119b165 100644 --- a/db.cc +++ b/db.cc @@ -52,6 +52,11 @@ DBC * Libdb::get_database(){ return this->cursorp; } +void Libdb::close_db() { + cursorp->close(cursorp); + db->close(db, 0); +} + GDBM_::GDBM_(){ database_type = DB_type::GDBM; } From 480baa2cce68fa8ddd05297e2cb6a65ee6999e54 Mon Sep 17 00:00:00 2001 From: Matej Matuska <75834032+matejmatuska@users.noreply.github.com> Date: Thu, 5 Sep 2024 11:47:26 +0200 Subject: [PATCH 5/6] Close databases in destructors --- convert.cc | 5 ----- convert_db.h | 5 ++++- db.cc | 21 +++++++++++++++------ 3 files changed, 19 insertions(+), 12 deletions(-) diff --git a/convert.cc b/convert.cc index 63377fe..d34f84d 100755 --- a/convert.cc +++ b/convert.cc @@ -67,20 +67,15 @@ int main(int argc, char* argv[]){ if (!b->create_database(dst)) { std::cerr<<"Failed to create destination database\n"; delete x; - x->close_db(); delete b; return 1; } if (!b->fill_database(x)){ std::cerr<<"database filling failed\n"; - x->close_db(); - b->close_db(); delete x; delete b; return 1; } - x->close_db(); - b->close_db(); delete x; delete b; return 0; diff --git a/convert_db.h b/convert_db.h index e11b8d3..9e8cf25 100644 --- a/convert_db.h +++ b/convert_db.h @@ -37,7 +37,7 @@ class DB_ { virtual bool create_database(std::string name); bool create_db(); virtual bool fill_database(DB_ * old_database); - virtual void close_db(); + virtual void close_db() = 0; virtual DBC * get_database(); virtual ~DB_(); }; @@ -55,6 +55,7 @@ class Libdb: public DB_ { bool connect_database(std::string path); DBC * get_database(); void close_db(); + ~Libdb(); }; /* * GDBM class provides API for GDBM, allowes to open and create and fill gdbm database @@ -71,6 +72,7 @@ class GDBM_: public DB_ { bool create_database(std::string name); bool fill_database(DB_ * old_database); void close_db(); + ~GDBM_(); }; //https://github.com/LMDB/lmdb/blob/mdb.master/libraries/liblmdb/mtest2.c //further inspiration @@ -88,5 +90,6 @@ class LMDB_: public DB_ { bool create_database(std::string name); bool fill_database(DB_ * old_database); void close_db(); + ~LMDB_(); }; diff --git a/db.cc b/db.cc index 119b165..cf7b9a1 100644 --- a/db.cc +++ b/db.cc @@ -1,10 +1,5 @@ #include "convert_db.h" - - -Libdb::Libdb(){ - database_type = DB_type::LIBDB; -}; bool DB_::connect_database(std::string){ return false; } @@ -25,6 +20,10 @@ void DB_::close_db(){} DB_::~DB_() { } +Libdb::Libdb(){ + database_type = DB_type::LIBDB; +}; + bool Libdb::connect_database(std::string path){ DB * db; int status; @@ -57,6 +56,10 @@ void Libdb::close_db() { db->close(db, 0); } +Libdb::~Libdb() { + close_db(); +} + GDBM_::GDBM_(){ database_type = DB_type::GDBM; } @@ -97,6 +100,10 @@ void GDBM_::close_db(){ gdbm_close(this->f); } +GDBM_::~GDBM_() { + close_db(); +} + LMDB_::LMDB_(){ database_type = DB_type::LMDB; @@ -170,4 +177,6 @@ void LMDB_::close_db(){ mdb_env_close(this->lmdb_database); } - +LMDB_::~LMDB_() { + close_db(); +} From 766a358ba617711171847f4b9038e8867f564a2a Mon Sep 17 00:00:00 2001 From: Matej Matuska <75834032+matejmatuska@users.noreply.github.com> Date: Thu, 5 Sep 2024 11:47:50 +0200 Subject: [PATCH 6/6] Free allocated memory in GDBM_::fill_database --- db.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/db.cc b/db.cc index cf7b9a1..ee8962c 100644 --- a/db.cc +++ b/db.cc @@ -93,6 +93,8 @@ bool GDBM_::fill_database(DB_ * old_database){ if (status != 0) return false; } + free(data_db); + free(key_db); return true; }