Merge bitcoin/bitcoin#25971: refactor: Use std::string for thread and index names

26cf9ea8e44d7fd6450336f567afaedd1275baf7 scripted-diff: rename pszThread to thread_name (stickies-v)
200d84d5681918523d982b9ddf60d1127edcb448 refactor: use std::string for index names (stickies-v)
97f5b20c12ca6ccf89d7720a5d41eaf4cda1b695 refactor: use std::string for thread names (stickies-v)

Pull request description:

  As a follow-up to https://github.com/bitcoin/bitcoin/pull/25967#discussion_r959637189, this PR changes the return type of [`BaseIndex::GetName()`](https://github.com/bitcoin/bitcoin/blob/fa5c224d444802dabec5841009e029b9754c92f1/src/index/base.h#L120) to `const std::string&` instead of `const char*`. The first commit is not essential for this change, but since the code is touched and index names are commonly used to specify thread names, I've made the same update there.

  No behaviour change, just refactoring to further phase out C-style strings.

  Note: `util::ThreadRename()` used to take an rvalue ref, but since it then passes this to `SetInternalName()` by value, I don't think there's any benefit to having both an rvalue and lvalue ref function so I just changed it into lvalue ref. Not 100% sure I'm missing something?

ACKs for top commit:
  MarcoFalke:
    review ACK 26cf9ea8e44d7fd6450336f567afaedd1275baf7 only change is new scripted-diff 😀
  hebasto:
    ACK 26cf9ea8e44d7fd6450336f567afaedd1275baf7, I have reviewed the code and it looks OK.
  w0xlt:
    reACK https://github.com/bitcoin/bitcoin/pull/25971/commits/26cf9ea8e44d7fd6450336f567afaedd1275baf7

Tree-SHA512: 44a03ebf2bb86ca1411a36222a575217cdba8ee3a3c985e74d74c934516f002b27336147fa22f59eda7dac21204a93951563317005d475da95b23c427014d77b

13 files changed, 27 insertions, 26 deletions

diff --git a/src/index/base.cpp b/src/index/base.cpp
index 1ebe89ef7c..88c2ce98fa 100644
--- a/src/index/base.cpp
+++ b/src/index/base.cpp
@@ -18,6 +18,9 @@
 #include <validation.h> // For g_chainman
 #include <warnings.h>
 
+#include <string>
+#include <utility>
+
 using node::ReadBlockFromDisk;
 
 constexpr uint8_t DB_BEST_BLOCK{'B'};
@@ -62,8 +65,8 @@ void BaseIndex::DB::WriteBestBlock(CDBBatch& batch, const CBlockLocator& locator
     batch.Write(DB_BEST_BLOCK, locator);
 }
 
-BaseIndex::BaseIndex(std::unique_ptr<interfaces::Chain> chain)
-    : m_chain{std::move(chain)} {}
+BaseIndex::BaseIndex(std::unique_ptr<interfaces::Chain> chain, std::string name)
+    : m_chain{std::move(chain)}, m_name{std::move(name)} {}
 
 BaseIndex::~BaseIndex()
 {
diff --git a/src/index/base.h b/src/index/base.h
index 14693bc6fe..349178a535 100644
--- a/src/index/base.h
+++ b/src/index/base.h
@@ -10,6 +10,8 @@
 #include <threadinterrupt.h>
 #include <validationinterface.h>
 
+#include <string>
+
 class CBlock;
 class CBlockIndex;
 class Chainstate;
@@ -95,6 +97,7 @@ private:
 protected:
     std::unique_ptr<interfaces::Chain> m_chain;
     Chainstate* m_chainstate{nullptr};
+    const std::string m_name;
 
     void BlockConnected(const std::shared_ptr<const CBlock>& block, const CBlockIndex* pindex) override;
 
@@ -117,13 +120,13 @@ protected:
     virtual DB& GetDB() const = 0;
 
     /// Get the name of the index for display in logs.
-    virtual const char* GetName() const = 0;
+    const std::string& GetName() const LIFETIMEBOUND { return m_name; }
 
     /// Update the internal best block index as well as the prune lock.
     void SetBestBlockIndex(const CBlockIndex* block);
 
 public:
-    BaseIndex(std::unique_ptr<interfaces::Chain> chain);
+    BaseIndex(std::unique_ptr<interfaces::Chain> chain, std::string name);
     /// Destructor interrupts sync thread if running and blocks until it exits.
     virtual ~BaseIndex();
 
diff --git a/src/index/blockfilterindex.cpp b/src/index/blockfilterindex.cpp
index f4837f3456..292e11c874 100644
--- a/src/index/blockfilterindex.cpp
+++ b/src/index/blockfilterindex.cpp
@@ -97,7 +97,8 @@ static std::map<BlockFilterType, BlockFilterIndex> g_filter_indexes;
 
 BlockFilterIndex::BlockFilterIndex(std::unique_ptr<interfaces::Chain> chain, BlockFilterType filter_type,
                                    size_t n_cache_size, bool f_memory, bool f_wipe)
-    : BaseIndex(std::move(chain)), m_filter_type(filter_type)
+    : BaseIndex(std::move(chain), BlockFilterTypeName(filter_type) + " block filter index")
+    , m_filter_type(filter_type)
 {
     const std::string& filter_name = BlockFilterTypeName(filter_type);
     if (filter_name.empty()) throw std::invalid_argument("unknown filter_type");
@@ -105,7 +106,6 @@ BlockFilterIndex::BlockFilterIndex(std::unique_ptr<interfaces::Chain> chain, Blo
     fs::path path = gArgs.GetDataDirNet() / "indexes" / "blockfilter" / fs::u8path(filter_name);
     fs::create_directories(path);
 
-    m_name = filter_name + " block filter index";
     m_db = std::make_unique<BaseIndex::DB>(path / "db", n_cache_size, f_memory, f_wipe);
     m_filter_fileseq = std::make_unique<FlatFileSeq>(std::move(path), "fltr", FLTR_FILE_CHUNK_SIZE);
 }
diff --git a/src/index/blockfilterindex.h b/src/index/blockfilterindex.h
index a31f7e460e..5af4671091 100644
--- a/src/index/blockfilterindex.h
+++ b/src/index/blockfilterindex.h
@@ -26,7 +26,6 @@ class BlockFilterIndex final : public BaseIndex
 {
 private:
     BlockFilterType m_filter_type;
-    std::string m_name;
     std::unique_ptr<BaseIndex::DB> m_db;
 
     FlatFilePos m_next_filter_pos;
@@ -52,8 +51,6 @@ protected:
 
     BaseIndex::DB& GetDB() const LIFETIMEBOUND override { return *m_db; }
 
-    const char* GetName() const LIFETIMEBOUND override { return m_name.c_str(); }
-
 public:
     /** Constructs the index, which becomes available to be queried. */
     explicit BlockFilterIndex(std::unique_ptr<interfaces::Chain> chain, BlockFilterType filter_type,
diff --git a/src/index/coinstatsindex.cpp b/src/index/coinstatsindex.cpp
index 99a1310c9e..d3559b1b75 100644
--- a/src/index/coinstatsindex.cpp
+++ b/src/index/coinstatsindex.cpp
@@ -105,7 +105,7 @@ struct DBHashKey {
 std::unique_ptr<CoinStatsIndex> g_coin_stats_index;
 
 CoinStatsIndex::CoinStatsIndex(std::unique_ptr<interfaces::Chain> chain, size_t n_cache_size, bool f_memory, bool f_wipe)
-    : BaseIndex(std::move(chain))
+    : BaseIndex(std::move(chain), "coinstatsindex")
 {
     fs::path path{gArgs.GetDataDirNet() / "indexes" / "coinstats"};
     fs::create_directories(path);
diff --git a/src/index/coinstatsindex.h b/src/index/coinstatsindex.h
index 7375a85750..fa59cb1ab1 100644
--- a/src/index/coinstatsindex.h
+++ b/src/index/coinstatsindex.h
@@ -20,7 +20,6 @@ struct CCoinsStats;
 class CoinStatsIndex final : public BaseIndex
 {
 private:
-    std::string m_name;
     std::unique_ptr<BaseIndex::DB> m_db;
 
     MuHash3072 m_muhash;
@@ -52,8 +51,6 @@ protected:
 
     BaseIndex::DB& GetDB() const override { return *m_db; }
 
-    const char* GetName() const override { return "coinstatsindex"; }
-
 public:
     // Constructs the index, which becomes available to be queried.
     explicit CoinStatsIndex(std::unique_ptr<interfaces::Chain> chain, size_t n_cache_size, bool f_memory = false, bool f_wipe = false);
diff --git a/src/index/txindex.cpp b/src/index/txindex.cpp
index b719aface8..a4fe1b611e 100644
--- a/src/index/txindex.cpp
+++ b/src/index/txindex.cpp
@@ -49,7 +49,7 @@ bool TxIndex::DB::WriteTxs(const std::vector<std::pair<uint256, CDiskTxPos>>& v_
 }
 
 TxIndex::TxIndex(std::unique_ptr<interfaces::Chain> chain, size_t n_cache_size, bool f_memory, bool f_wipe)
-    : BaseIndex(std::move(chain)), m_db(std::make_unique<TxIndex::DB>(n_cache_size, f_memory, f_wipe))
+    : BaseIndex(std::move(chain), "txindex"), m_db(std::make_unique<TxIndex::DB>(n_cache_size, f_memory, f_wipe))
 {}
 
 TxIndex::~TxIndex() = default;
diff --git a/src/index/txindex.h b/src/index/txindex.h
index be240c4582..8c1aa00033 100644
--- a/src/index/txindex.h
+++ b/src/index/txindex.h
@@ -27,8 +27,6 @@ protected:
 
     BaseIndex::DB& GetDB() const override;
 
-    const char* GetName() const override { return "txindex"; }
-
 public:
     /// Constructs the index, which becomes available to be queried.
     explicit TxIndex(std::unique_ptr<interfaces::Chain> chain, size_t n_cache_size, bool f_memory = false, bool f_wipe = false);
diff --git a/src/qt/guiutil.cpp b/src/qt/guiutil.cpp
index 5cc21dd40b..b9f0be41e3 100644
--- a/src/qt/guiutil.cpp
+++ b/src/qt/guiutil.cpp
@@ -982,7 +982,7 @@ void PrintSlotException(
     std::string description = sender->metaObject()->className();
     description += "->";
     description += receiver->metaObject()->className();
-    PrintExceptionContinue(exception, description.c_str());
+    PrintExceptionContinue(exception, description);
 }
 
 void ShowModalDialogAsynchronously(QDialog* dialog)
diff --git a/src/util/system.cpp b/src/util/system.cpp
index 1953a9f836..c3c6cbfef6 100644
--- a/src/util/system.cpp
+++ b/src/util/system.cpp
@@ -831,7 +831,7 @@ std::string HelpMessageOpt(const std::string &option, const std::string &message
            std::string("\n\n");
 }
 
-static std::string FormatException(const std::exception* pex, const char* pszThread)
+static std::string FormatException(const std::exception* pex, std::string_view thread_name)
 {
 #ifdef WIN32
     char pszModule[MAX_PATH] = "";
@@ -841,15 +841,15 @@ static std::string FormatException(const std::exception* pex, const char* pszThr
 #endif
     if (pex)
         return strprintf(
-            "EXCEPTION: %s       \n%s       \n%s in %s       \n", typeid(*pex).name(), pex->what(), pszModule, pszThread);
+            "EXCEPTION: %s       \n%s       \n%s in %s       \n", typeid(*pex).name(), pex->what(), pszModule, thread_name);
     else
         return strprintf(
-            "UNKNOWN EXCEPTION       \n%s in %s       \n", pszModule, pszThread);
+            "UNKNOWN EXCEPTION       \n%s in %s       \n", pszModule, thread_name);
 }
 
-void PrintExceptionContinue(const std::exception* pex, const char* pszThread)
+void PrintExceptionContinue(const std::exception* pex, std::string_view thread_name)
 {
-    std::string message = FormatException(pex, pszThread);
+    std::string message = FormatException(pex, thread_name);
     LogPrintf("\n\n************************\n%s\n", message);
     tfm::format(std::cerr, "\n\n************************\n%s\n", message);
 }
diff --git a/src/util/system.h b/src/util/system.h
index 756e6642f2..c8e1de6700 100644
--- a/src/util/system.h
+++ b/src/util/system.h
@@ -51,7 +51,7 @@ bool error(const char* fmt, const Args&... args)
     return false;
 }
 
-void PrintExceptionContinue(const std::exception *pex, const char* pszThread);
+void PrintExceptionContinue(const std::exception* pex, std::string_view thread_name);
 
 /**
  * Ensure file contents are fully committed to disk, using a platform-specific
diff --git a/src/util/thread.cpp b/src/util/thread.cpp
index f9f427ba20..ae98abdb3d 100644
--- a/src/util/thread.cpp
+++ b/src/util/thread.cpp
@@ -10,10 +10,12 @@
 
 #include <exception>
 #include <functional>
+#include <string>
+#include <utility>
 
-void util::TraceThread(const char* thread_name, std::function<void()> thread_func)
+void util::TraceThread(std::string_view thread_name, std::function<void()> thread_func)
 {
-    util::ThreadRename(thread_name);
+    util::ThreadRename(std::string{thread_name});
     try {
         LogPrintf("%s thread start\n", thread_name);
         thread_func();
diff --git a/src/util/thread.h b/src/util/thread.h
index ca2eccc0c3..b80bf046a0 100644
--- a/src/util/thread.h
+++ b/src/util/thread.h
@@ -6,12 +6,13 @@
 #define BITCOIN_UTIL_THREAD_H
 
 #include <functional>
+#include <string>
 
 namespace util {
 /**
  * A wrapper for do-something-once thread functions.
  */
-void TraceThread(const char* thread_name, std::function<void()> thread_func);
+void TraceThread(std::string_view thread_name, std::function<void()> thread_func);
 
 } // namespace util