Berkeley DB v6 compatibility fix #8626

netsafe commented at 6:01 AM on August 30, 2016: none

Fixes building error looking like this: CXX wallet/libbitcoin_wallet_a-db.o wallet/db.cpp: In member function ‘void CDBEnv::EnvShutdown()’: wallet/db.cpp:46:16: error: call of overloaded ‘DbEnv(int)’ is ambiguous DbEnv(0).remove(strPath.c_str(), 0); ^ wallet/db.cpp:46:16: note: candidates are: In file included from wallet/db.h:21:0, from wallet/db.cpp:6: /usr/include/db_cxx.h:916:2: note: DbEnv::DbEnv(const DbEnv&) DbEnv(const DbEnv &); ^ /usr/include/db_cxx.h:518:2: note: DbEnv::DbEnv(DB_ENV) DbEnv(DB_ENV dbenv); ^ /usr/include/db_cxx.h:516:2: note: DbEnv::DbEnv(u_int32_t) DbEnv(u_int32_t flags); ^ Makefile:5780: recipe for target 'wallet/libbitcoin_wallet_a-db.o' failed make[2]: * [wallet/libbitcoin_wallet_a-db.o] Error 1

Berkeley DB v6 compatibility fix

Fixes building error looking like this:
CXX wallet/libbitcoin_wallet_a-db.o
wallet/db.cpp: In member function ‘void CDBEnv::EnvShutdown()’:
wallet/db.cpp:46:16: error: call of overloaded ‘DbEnv(int)’ is ambiguous
DbEnv(0).remove(strPath.c_str(), 0);
^
wallet/db.cpp:46:16: note: candidates are:
In file included from wallet/db.h:21:0,
from wallet/db.cpp:6:
/usr/include/db_cxx.h:916:2: note: DbEnv::DbEnv(const DbEnv&)
DbEnv(const DbEnv &);
^
/usr/include/db_cxx.h:518:2: note: DbEnv::DbEnv(DB_ENV)
DbEnv(DB_ENV *dbenv);
^
/usr/include/db_cxx.h:516:2: note: DbEnv::DbEnv(u_int32_t)
DbEnv(u_int32_t flags);
^
Makefile:5780: recipe for target 'wallet/libbitcoin_wallet_a-db.o' failed
make[2]: ** [wallet/libbitcoin_wallet_a-db.o] Error 1

323a5fe06a

netsafe commented at 6:09 AM on August 30, 2016: none

I've originally found it for namecoin - all the detalis there https://github.com/namecoin/namecoin-core/pull/101

jonasschnelli added the label Wallet on Aug 30, 2016

jonasschnelli added the label Build system on Aug 30, 2016

jonasschnelli commented at 6:38 AM on August 30, 2016: contributor

I think this change makes sense. BDB upstream documentation for the DbEnv constructor: https://docs.oracle.com/cd/E17276_01/html/api_reference/CXX/envcreate.html

luke-jr commented at 7:21 AM on August 30, 2016: member

It might actually be a good idea to fail to build against BDB 6... it changed the license to AGPL, and so users will by default be infringing if they just use Bitcoin Core as-is this way (as AGPL requires providing source code to every peer).

That said, I don't really care strongly either way. Perhaps a clear warning somewhere would be better.

netsafe commented at 7:25 AM on August 30, 2016: none

@luke-jr and the code is open to every peer, actually - the peer is running an open-source Bitcoin core. What exact do you mean by infringing the license? I see no license is broken here http://www.oracle.com/technetwork/database/berkeleydb/downloads/oslicense-093458.html

luke-jr commented at 7:33 AM on August 30, 2016: member

@netsafe It's not enough that someone else is sharing the code; the person running the software would need to do so, or at least provide notice to each peer how to get it. Right now, Bitcoin Core doesn't do even the notice. That being said, skimming over the AGPL again it seems this requirement exists only for modified versions, so it's probably less of an issue than it seems at face value. But IANAL and it seems like not the best idea to have the license transparently change like this.

In any case, I still don't really care strongly either way. :)

netsafe commented at 7:41 AM on August 30, 2016: none

@luke-jr I'm not caring that deep too. As far as I understood this situation, AGPL will fire the way we're speaking only if it's a client-server architecture. But in a P2P case - which is a case for Bitcoin - all peers not just should have the same source code, but they're explicitly obliged to : or the network will fall apart because it's nodes won't be able to work together in the same way.

luke-jr commented at 7:45 AM on August 30, 2016: member

P2P is just "everyone is a client and server to each other". It's stronger with AGPL, not weaker.

There is absolutely no requirement for all nodes to be running the same source code, only that they be using the same consensus logic, which is a subset of the full program (and doesn't touch the BDB-related code at all).

laanwj commented at 8:06 AM on August 30, 2016: member

It might actually be a good idea to fail to build against BDB 6...

Let's not get involved in copyfights here but focus on the technical side - apparently someone wants to fix building with BDB 6, and if the code changes are acceptable (and don't regress BDB 4/5 support), let's just accept that.

netsafe commented at 8:24 AM on August 30, 2016: none

@laanwj It does not breaks 4/5 BDB support - test build confirms it too

Official docs http://docs.oracle.com/cd/E17076_02/html/api_reference/CXX/BDB-CXX_APIReference.pdf page 226 :

#include <db_cxx.h>
class DbEnv { public: DbEnv(u_int32 flags); ~DbEnv(); DB_ENV *DbEnv::get_DB_ENV(); const DB_ENV *DbEnv::get_const_DB_ENV() const; static DbEnv *DbEnv::get_DbEnv(DB_ENV *dbenv); static const DbEnv *DbEnv::get_const_DbEnv(const DB_ENV *dbenv); ... };

in src/wallet/db.cpp:None in 323a5fe06a

  42 | @@ -43,7 +43,7 @@ void CDBEnv::EnvShutdown()
  43 |      if (ret != 0)
  44 |          LogPrintf("CDBEnv::EnvShutdown: Error %d shutting down database environment: %s\n", ret, DbEnv::strerror(ret));
  45 |      if (!fMockDb)
  46 | -        DbEnv(0).remove(strPath.c_str(), 0);
  47 | +        DbEnv((u_int32_t)0).remove(strPath.c_str(), 0);

laanwj commented at 8:42 AM on August 30, 2016:

Would the c++11 keyword nullptr work here?

netsafe commented at 8:50 AM on August 30, 2016:

No, it must be zero and explicitly of type u_int32_t : "The flags parameter must be set to 0." in the docs it's stated strictly

laanwj commented at 10:18 AM on August 30, 2016:

Oh right! I was misreading, you're not casting 0 to a pointer.

paveljanik commented at 9:01 AM on August 30, 2016: contributor

ACK https://github.com/bitcoin/bitcoin/pull/8626/commits/323a5fe06af43d3922a435e696d6c52acc6fade1

netsafe commented at 9:09 AM on August 30, 2016: none

the configuration string that I'm using for Berkeley DB is - in build_unix folder :

../dist/configure --prefix=/usr --enable-dbm --enable-cxx --enable-compat185 --enable-server --enable-o_direct --with-cryptography=yes --enable-sql --enable-sql_compat --enable-sql_codegen --enable-stl --enable-localization

versions 6.2.23, 5.3.28 and 4.8.30 are producing db_cxx.h that contains the same definition

laanwj commented at 10:19 AM on August 30, 2016: member

utACK https://github.com/bitcoin/bitcoin/pull/8626/commits/323a5fe06af43d3922a435e696d6c52acc6fade1

sipa commented at 1:14 PM on August 30, 2016: member

utACK 323a5fe06af43d3922a435e696d6c52acc6fade1

laanwj merged this on Aug 31, 2016

laanwj closed this on Aug 31, 2016

laanwj referenced this in commit df3f12d409 on Aug 31, 2016

luke-jr referenced this in commit ea51b0f5dd on Sep 21, 2016

codablock referenced this in commit 53f84ce101 on Sep 19, 2017

codablock referenced this in commit e5702cd6aa on Jan 9, 2018

codablock referenced this in commit 3b45caeeb7 on Jan 9, 2018

andvgal referenced this in commit 0421462575 on Jan 6, 2019

Fuzzbawls referenced this in commit d752ee1388 on Mar 5, 2021

DrahtBot locked this on Sep 8, 2021