Merge #12630: Provide useful error message if datadir is not writable.

8674e74 Provide relevant error message if datadir is not writable. (murrayn)

Pull request description:

  If the --datadir exists, but is not writable, the current error message on startup is 'Cannot obtain a lock on data directory foo. Bitcoin Core is probably already running.' This is misleading.

  I believe this PR addresses #11668, although the issue is not Windows-specific.

Tree-SHA512: 10cbbaea433072aee4fb3e8938a72073c7a5c841f7a7685c9e12549c322b2925c7d34bac254ac33021b23132bfc352c058712bc9542298cf86f8fd9757f528b2

4 files changed, 33 insertions, 0 deletions

diff --git a/src/init.cpp b/src/init.cpp
index e0efe5c0a2..30ee0e694c 100644
--- a/src/init.cpp
+++ b/src/init.cpp
@@ -1165,6 +1165,9 @@ static bool LockDataDirectory(bool probeOnly)
 {
     // Make sure only a single Bitcoin process is using the data directory.
     fs::path datadir = GetDataDir();
+    if (!DirIsWritable(datadir)) {
+        return InitError(strprintf(_("Cannot write to data directory '%s'; check permissions."), datadir.string()));
+    }
     if (!LockDirectory(datadir, ".lock", probeOnly)) {
         return InitError(strprintf(_("Cannot obtain a lock on data directory %s. %s is probably already running."), datadir.string(), _(PACKAGE_NAME)));
     }
diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp
index 84b61bea86..b6f3cbe2b7 100644
--- a/src/test/util_tests.cpp
+++ b/src/test/util_tests.cpp
@@ -817,4 +817,20 @@ BOOST_AUTO_TEST_CASE(test_LockDirectory)
     fs::remove_all(dirname);
 }
 
+BOOST_AUTO_TEST_CASE(test_DirIsWritable)
+{
+    // Should be able to write to the system tmp dir.
+    fs::path tmpdirname = fs::temp_directory_path();
+    BOOST_CHECK_EQUAL(DirIsWritable(tmpdirname), true);
+
+    // Should not be able to write to a non-existent dir.
+    tmpdirname = fs::temp_directory_path() / fs::unique_path();
+    BOOST_CHECK_EQUAL(DirIsWritable(tmpdirname), false);
+
+    fs::create_directory(tmpdirname);
+    // Should be able to write to it now.
+    BOOST_CHECK_EQUAL(DirIsWritable(tmpdirname), true);
+    fs::remove(tmpdirname);
+}
+
 BOOST_AUTO_TEST_SUITE_END()
diff --git a/src/util.cpp b/src/util.cpp
index 69ceefc8cd..494d5c4eaf 100644
--- a/src/util.cpp
+++ b/src/util.cpp
@@ -419,6 +419,19 @@ void ReleaseDirectoryLocks()
     dir_locks.clear();
 }
 
+bool DirIsWritable(const fs::path& directory)
+{
+    fs::path tmpFile = directory / fs::unique_path();
+
+    FILE* file = fsbridge::fopen(tmpFile, "a");
+    if (!file) return false;
+
+    fclose(file);
+    remove(tmpFile);
+
+    return true;
+}
+
 /** Interpret string as boolean, for argument parsing */
 static bool InterpretBool(const std::string& strValue)
 {
diff --git a/src/util.h b/src/util.h
index aebd2fd590..04ff44f218 100644
--- a/src/util.h
+++ b/src/util.h
@@ -174,6 +174,7 @@ int RaiseFileDescriptorLimit(int nMinFD);
 void AllocateFileRange(FILE *file, unsigned int offset, unsigned int length);
 bool RenameOver(fs::path src, fs::path dest);
 bool LockDirectory(const fs::path& directory, const std::string lockfile_name, bool probe_only=false);
+bool DirIsWritable(const fs::path& directory);
 
 /** Release all directory locks. This is used for unit testing only, at runtime
  * the global destructor will take care of the locks.