Merge bitcoin/bitcoin#28075: util: Remove DirIsWritable, GetUniquePath

fa3da629a1 Remove DirIsWritable, GetUniquePath (MarcoFalke)
fad3a9793b Return LockResult::ErrorWrite in LockDirectory (MarcoFalke)
fa0afe7408 refactor: Return enum in LockDirectory (MarcoFalke)

Pull request description:

  `GetUniquePath` is only used in tests and in `DirIsWritable`. The check by `DirIsWritable` is redundant with the check done in `LockDirectory`.

  Fix the redundancy by removing everything, except `LockDirectory`.

ACKs for top commit:
  TheCharlatan:
    Re-ACK fa3da629a1
  hebasto:
    ACK fa3da629a1, I have reviewed the code and it looks OK.

Tree-SHA512: e95f18cd586de7582e9c08ac7ddb860bfcfcbc8963804f45c5784c5e4c0598dc59ae7e45dd4daf30a5020dbf8433f5db2ad06e46a8676371982003790043c6c9

src/util/fs_helpers.cpp

@@ -12,7 +12,6 @@
 #include <logging.h>
 #include <sync.h>
 #include <util/fs.h>
-#include <util/getuniquepath.h>
 #include <util/syserror.h>
 
 #include <cerrno>
@@ -51,31 +50,35 @@ static GlobalMutex cs_dir_locks;
  * is called.
  */
 static std::map<std::string, std::unique_ptr<fsbridge::FileLock>> dir_locks GUARDED_BY(cs_dir_locks);
-
-bool LockDirectory(const fs::path& directory, const fs::path& lockfile_name, bool probe_only)
+namespace util {
+LockResult LockDirectory(const fs::path& directory, const fs::path& lockfile_name, bool probe_only)
 {
     LOCK(cs_dir_locks);
     fs::path pathLockFile = directory / lockfile_name;
 
     // If a lock for this directory already exists in the map, don't try to re-lock it
     if (dir_locks.count(fs::PathToString(pathLockFile))) {
-        return true;
+        return LockResult::Success;
     }
 
     // Create empty lock file if it doesn't exist.
-    FILE* file = fsbridge::fopen(pathLockFile, "a");
-    if (file) fclose(file);
+    if (auto created{fsbridge::fopen(pathLockFile, "a")}) {
+        std::fclose(created);
+    } else {
+        return LockResult::ErrorWrite;
+    }
     auto lock = std::make_unique<fsbridge::FileLock>(pathLockFile);
     if (!lock->TryLock()) {
-        return error("Error while attempting to lock directory %s: %s", fs::PathToString(directory), lock->GetReason());
+        error("Error while attempting to lock directory %s: %s", fs::PathToString(directory), lock->GetReason());
+        return LockResult::ErrorLock;
     }
     if (!probe_only) {
         // Lock successful and we're not just probing, put it into the map
         dir_locks.emplace(fs::PathToString(pathLockFile), std::move(lock));
     }
-    return true;
+    return LockResult::Success;
 }
-
+} // namespace util
 void UnlockDirectory(const fs::path& directory, const fs::path& lockfile_name)
 {
     LOCK(cs_dir_locks);
@@ -88,19 +91,6 @@ void ReleaseDirectoryLocks()
     dir_locks.clear();
 }
 
-bool DirIsWritable(const fs::path& directory)
-{
-    fs::path tmpFile = GetUniquePath(directory);
-
-    FILE* file = fsbridge::fopen(tmpFile, "a");
-    if (!file) return false;
-
-    fclose(file);
-    remove(tmpFile);
-
-    return true;
-}
-
 bool CheckDiskSpace(const fs::path& dir, uint64_t additional_bytes)
 {
     constexpr uint64_t min_disk_space = 52428800; // 50 MiB

src/util/fs_helpers.h

@@ -35,9 +35,15 @@ void AllocateFileRange(FILE* file, unsigned int offset, unsigned int length);
  */
 [[nodiscard]] bool RenameOver(fs::path src, fs::path dest);
 
-bool LockDirectory(const fs::path& directory, const fs::path& lockfile_name, bool probe_only = false);
+namespace util {
+enum class LockResult {
+    Success,
+    ErrorWrite,
+    ErrorLock,
+};
+[[nodiscard]] LockResult LockDirectory(const fs::path& directory, const fs::path& lockfile_name, bool probe_only = false);
+} // namespace util
 void UnlockDirectory(const fs::path& directory, const fs::path& lockfile_name);
-bool DirIsWritable(const fs::path& directory);
 bool CheckDiskSpace(const fs::path& dir, uint64_t additional_bytes = 0);
 
 /** Get the size of a file by scanning it.

src/util/getuniquepath.cpp

@@ -1,14 +0,0 @@
-// Copyright (c) 2021-2022 The Bitcoin Core developers
-// Distributed under the MIT software license, see the accompanying
-// file COPYING or http://www.opensource.org/licenses/mit-license.php.
-
-#include <random.h>
-#include <util/fs.h>
-#include <util/strencodings.h>
-
-fs::path GetUniquePath(const fs::path& base)
-{
-    FastRandomContext rnd;
-    fs::path tmpFile = base / fs::u8path(HexStr(rnd.randbytes(8)));
-    return tmpFile;
-}

src/util/getuniquepath.h

@@ -1,19 +0,0 @@
-// Copyright (c) 2021 The Bitcoin Core developers
-// Distributed under the MIT software license, see the accompanying
-// file COPYING or http://www.opensource.org/licenses/mit-license.php.
-
-#ifndef BITCOIN_UTIL_GETUNIQUEPATH_H
-#define BITCOIN_UTIL_GETUNIQUEPATH_H
-
-#include <util/fs.h>
-
-/**
- * Helper function for getting a unique path
- *
- * @param[in] base  Base path
- * @returns base joined with a random 8-character long string.
- * @post Returned path is unique with high probability.
- */
-fs::path GetUniquePath(const fs::path& base);
-
-#endif // BITCOIN_UTIL_GETUNIQUEPATH_H