From c1f325126cf51d28dce8da74bfdf5cd05ab237ea Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Mon, 29 Apr 2019 22:46:34 +0300 Subject: [PATCH 1/7] Return absolute path early in AbsPathForConfigVal This prevents premature GetDataDir() calls, e.g., when config file is not read yet. --- src/util/system.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/util/system.cpp b/src/util/system.cpp index 72b37b9187..d9e23199ec 100644 --- a/src/util/system.cpp +++ b/src/util/system.cpp @@ -1202,6 +1202,9 @@ int64_t GetStartupTime() fs::path AbsPathForConfigVal(const fs::path& path, bool net_specific) { + if (path.is_absolute()) { + return path; + } return fs::absolute(path, GetDataDir(net_specific)); } From 740d41ce9f7fdf209366e930bd0fdcc6b1bc6b79 Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Wed, 24 Jul 2019 03:21:25 +0300 Subject: [PATCH 2/7] Add CheckDataDirOption() function --- src/util/system.cpp | 11 +++++++++-- src/util/system.h | 2 ++ 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/src/util/system.cpp b/src/util/system.cpp index d9e23199ec..520ed35504 100644 --- a/src/util/system.cpp +++ b/src/util/system.cpp @@ -741,8 +741,9 @@ const fs::path &GetDataDir(bool fNetSpecific) // this function if (!path.empty()) return path; - if (gArgs.IsArgSet("-datadir")) { - path = fs::system_complete(gArgs.GetArg("-datadir", "")); + std::string datadir = gArgs.GetArg("-datadir", ""); + if (!datadir.empty()) { + path = fs::system_complete(datadir); if (!fs::is_directory(path)) { path = ""; return path; @@ -761,6 +762,12 @@ const fs::path &GetDataDir(bool fNetSpecific) return path; } +bool CheckDataDirOption() +{ + std::string datadir = gArgs.GetArg("-datadir", ""); + return datadir.empty() || fs::is_directory(fs::system_complete(datadir)); +} + void ClearDatadirCache() { LOCK(csPathCached); diff --git a/src/util/system.h b/src/util/system.h index dda9156488..9ed9f1f0df 100644 --- a/src/util/system.h +++ b/src/util/system.h @@ -83,6 +83,8 @@ fs::path GetDefaultDataDir(); // The blocks directory is always net specific. const fs::path &GetBlocksDir(); const fs::path &GetDataDir(bool fNetSpecific = true); +// Return true if -datadir option points to a valid directory or is not specified. +bool CheckDataDirOption(); /** Tests only */ void ClearDatadirCache(); fs::path GetConfigFile(const std::string& confPath); From 50824093bb2d68fe1393dfd636fab5f8795faa5d Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Wed, 24 Jul 2019 03:29:40 +0300 Subject: [PATCH 3/7] Fix datadir handling in bitcoind This prevents premature tries to access or create the default datadir. This is useful when the -datadir option is specified and the default datadir is unreachable. --- src/bitcoind.cpp | 3 +-- src/util/system.cpp | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/bitcoind.cpp b/src/bitcoind.cpp index 77367d6bb8..4037f6856d 100644 --- a/src/bitcoind.cpp +++ b/src/bitcoind.cpp @@ -92,8 +92,7 @@ static bool AppInit(int argc, char* argv[]) try { - if (!fs::is_directory(GetDataDir(false))) - { + if (!CheckDataDirOption()) { return InitError(strprintf("Specified data directory \"%s\" does not exist.\n", gArgs.GetArg("-datadir", ""))); } if (!gArgs.ReadConfigFiles(error, true)) { diff --git a/src/util/system.cpp b/src/util/system.cpp index 520ed35504..4d8aa9ed90 100644 --- a/src/util/system.cpp +++ b/src/util/system.cpp @@ -941,7 +941,7 @@ bool ArgsManager::ReadConfigFiles(std::string& error, bool ignore_invalid_keys) // If datadir is changed in .conf file: ClearDatadirCache(); - if (!fs::is_directory(GetDataDir(false))) { + if (!CheckDataDirOption()) { error = strprintf("specified data directory \"%s\" does not exist.", gArgs.GetArg("-datadir", "").c_str()); return false; } From b28dada37465c0a773cf08b0e6766f0081bcb943 Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Wed, 24 Jul 2019 18:54:32 +0300 Subject: [PATCH 4/7] Fix datadir handling in bitcoin-qt This prevents premature tries to access or create the default datadir. This is useful when the -datadir option is specified and the default datadir is unreachable. --- src/qt/bitcoin.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/qt/bitcoin.cpp b/src/qt/bitcoin.cpp index ed5d47cad7..545dcef14a 100644 --- a/src/qt/bitcoin.cpp +++ b/src/qt/bitcoin.cpp @@ -495,10 +495,9 @@ int GuiMain(int argc, char* argv[]) if (!Intro::pickDataDirectory(*node)) return EXIT_SUCCESS; - /// 6. Determine availability of data and blocks directory and parse bitcoin.conf + /// 6. Determine availability of data directory and parse bitcoin.conf /// - Do not call GetDataDir(true) before this step finishes - if (!fs::is_directory(GetDataDir(false))) - { + if (!CheckDataDirOption()) { node->initError(strprintf("Specified data directory \"%s\" does not exist.\n", gArgs.GetArg("-datadir", ""))); QMessageBox::critical(nullptr, PACKAGE_NAME, QObject::tr("Error: Specified data directory \"%1\" does not exist.").arg(QString::fromStdString(gArgs.GetArg("-datadir", "")))); From 7e33a18a34b1a9b0f115076c142661d6d30c0585 Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Wed, 24 Jul 2019 18:54:46 +0300 Subject: [PATCH 5/7] Fix datadir handling in bitcoin-cli This prevents premature tries to access or create the default datadir. This is useful when the -datadir option is specified and the default datadir is unreachable. --- src/bitcoin-cli.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/bitcoin-cli.cpp b/src/bitcoin-cli.cpp index d3419520a7..b5c9747f47 100644 --- a/src/bitcoin-cli.cpp +++ b/src/bitcoin-cli.cpp @@ -123,7 +123,7 @@ static int AppInitRPC(int argc, char* argv[]) } return EXIT_SUCCESS; } - if (!fs::is_directory(GetDataDir(false))) { + if (!CheckDataDirOption()) { tfm::format(std::cerr, "Error: Specified data directory \"%s\" does not exist.\n", gArgs.GetArg("-datadir", "").c_str()); return EXIT_FAILURE; } From 66f5c17f8a3fe06fc65191e379ffc04e43cbbc86 Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Wed, 24 Jul 2019 18:54:52 +0300 Subject: [PATCH 6/7] Use CheckDataDirOption() for code uniformity All other clients and tools use CheckDataDirOption() rather fs::is_directory(GetDataDir(false)) for the first datadir check. --- src/bitcoin-wallet.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/bitcoin-wallet.cpp b/src/bitcoin-wallet.cpp index cbb4ea750c..596e126a21 100644 --- a/src/bitcoin-wallet.cpp +++ b/src/bitcoin-wallet.cpp @@ -55,7 +55,7 @@ static bool WalletAppInit(int argc, char* argv[]) // check for printtoconsole, allow -debug LogInstance().m_print_to_console = gArgs.GetBoolArg("-printtoconsole", gArgs.GetBoolArg("-debug", false)); - if (!fs::is_directory(GetDataDir(false))) { + if (!CheckDataDirOption()) { tfm::format(std::cerr, "Error: Specified data directory \"%s\" does not exist.\n", gArgs.GetArg("-datadir", "").c_str()); return false; } From ffea41f5301d5582665cf10ba5c2b9547a1443de Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Wed, 24 Jul 2019 19:15:10 +0300 Subject: [PATCH 7/7] Enable all tests in feature_config_args.py --- test/functional/feature_config_args.py | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/test/functional/feature_config_args.py b/test/functional/feature_config_args.py index aae262344d..e5603b1bba 100755 --- a/test/functional/feature_config_args.py +++ b/test/functional/feature_config_args.py @@ -108,17 +108,15 @@ class ConfArgsTest(BitcoinTestFramework): f.write("datadir=" + new_data_dir + "\n") f.write(conf_file_contents) - # Temporarily disabled, because this test would access the user's home dir (~/.bitcoin) - #self.nodes[0].assert_start_raises_init_error(['-conf=' + conf_file], 'Error reading configuration file: specified data directory "' + new_data_dir + '" does not exist.') + self.nodes[0].assert_start_raises_init_error(['-conf=' + conf_file], 'Error: Error reading configuration file: specified data directory "' + new_data_dir + '" does not exist.') # Create the directory and ensure the config file now works os.mkdir(new_data_dir) - # Temporarily disabled, because this test would access the user's home dir (~/.bitcoin) - #self.start_node(0, ['-conf='+conf_file, '-wallet=w1']) - #self.stop_node(0) - #assert os.path.exists(os.path.join(new_data_dir, 'regtest', 'blocks')) - #if self.is_wallet_compiled(): - #assert os.path.exists(os.path.join(new_data_dir, 'regtest', 'wallets', 'w1')) + self.start_node(0, ['-conf='+conf_file, '-wallet=w1']) + self.stop_node(0) + assert os.path.exists(os.path.join(new_data_dir, 'regtest', 'blocks')) + if self.is_wallet_compiled(): + assert os.path.exists(os.path.join(new_data_dir, 'regtest', 'wallets', 'w1')) # Ensure command line argument overrides datadir in conf os.mkdir(new_data_dir_2)