Merge #19004: refactor: Replace const char* to std::string

c57f03ce1741b38af448bec7b22ab9f8ac21f067 refactor: Replace const char* to std::string (Calvin Kim)

Pull request description:

  Rationale: Addresses #19000
  Some functions should be returning std::string instead of const char*.
  This commit changes that.

  Main benefits/reasoning:

  1.  The functions never return nullptr, so returning a string makes code at call sites easier to review (reviewers don't have to read the source code to verify that a nullptr is never returned)
  2. All call sites convert to string anyway

ACKs for top commit:
  MarcoFalke:
    re-ACK c57f03ce17 (no changes since previous review) 🚃
  Empact:
    Fair enough, Code Review ACK https://github.com/bitcoin/bitcoin/pull/19004/commits/c57f03ce1741b38af448bec7b22ab9f8ac21f067
  practicalswift:
    ACK c57f03ce1741b38af448bec7b22ab9f8ac21f067 -- patch looks correct
  hebasto:
    re-ACK c57f03ce1741b38af448bec7b22ab9f8ac21f067

Tree-SHA512: 9ce99bb38fe399b54844315048204cafce0f27fd8f24cae357fa7ac6f5d8094d57bbf5f5c1f5878a65f2d35e4a3f95d527eb17f49250b690c591c0df86ca84fd

9 files changed, 24 insertions, 13 deletions

diff --git a/src/bitcoin-cli.cpp b/src/bitcoin-cli.cpp
index 45a586cd12..8d85789b4e 100644
--- a/src/bitcoin-cli.cpp
+++ b/src/bitcoin-cli.cpp
@@ -21,6 +21,7 @@
 #include <functional>
 #include <memory>
 #include <stdio.h>
+#include <string>
 #include <tuple>
 
 #include <event2/buffer.h>
@@ -158,7 +159,7 @@ struct HTTPReply
     std::string body;
 };
 
-static const char *http_errorstring(int code)
+static std::string http_errorstring(int code)
 {
     switch(code) {
 #if LIBEVENT_VERSION_NUMBER >= 0x02010300
diff --git a/src/core_read.cpp b/src/core_read.cpp
index df78c319ee..1c0a8a096d 100644
--- a/src/core_read.cpp
+++ b/src/core_read.cpp
@@ -19,6 +19,7 @@
 #include <boost/algorithm/string/split.hpp>
 
 #include <algorithm>
+#include <string>
 
 CScript ParseScript(const std::string& s)
 {
@@ -34,10 +35,9 @@ CScript ParseScript(const std::string& s)
             if (op < OP_NOP && op != OP_RESERVED)
                 continue;
 
-            const char* name = GetOpName(static_cast<opcodetype>(op));
-            if (strcmp(name, "OP_UNKNOWN") == 0)
+            std::string strName = GetOpName(static_cast<opcodetype>(op));
+            if (strName == "OP_UNKNOWN")
                 continue;
-            std::string strName(name);
             mapOpNames[strName] = static_cast<opcodetype>(op);
             // Convenience: OP_ADD and just ADD are both recognized:
             boost::algorithm::replace_first(strName, "OP_", "");
diff --git a/src/script/script.cpp b/src/script/script.cpp
index ae0de1d24e..92c6fe7785 100644
--- a/src/script/script.cpp
+++ b/src/script/script.cpp
@@ -7,7 +7,9 @@
 
 #include <util/strencodings.h>
 
-const char* GetOpName(opcodetype opcode)
+#include <string>
+
+std::string GetOpName(opcodetype opcode)
 {
     switch (opcode)
     {
diff --git a/src/script/script.h b/src/script/script.h
index b61581767f..c1f2b66921 100644
--- a/src/script/script.h
+++ b/src/script/script.h
@@ -193,7 +193,7 @@ enum opcodetype
 // Maximum value that an opcode can be
 static const unsigned int MAX_OPCODE = OP_NOP10;
 
-const char* GetOpName(opcodetype opcode);
+std::string GetOpName(opcodetype opcode);
 
 class scriptnum_error : public std::runtime_error
 {
diff --git a/src/script/script_error.cpp b/src/script/script_error.cpp
index 57e8fee539..69e14803f1 100644
--- a/src/script/script_error.cpp
+++ b/src/script/script_error.cpp
@@ -5,7 +5,9 @@
 
 #include <script/script_error.h>
 
-const char* ScriptErrorString(const ScriptError serror)
+#include <string>
+
+std::string ScriptErrorString(const ScriptError serror)
 {
     switch (serror)
     {
diff --git a/src/script/script_error.h b/src/script/script_error.h
index 400f63ff0f..2978c147e1 100644
--- a/src/script/script_error.h
+++ b/src/script/script_error.h
@@ -6,6 +6,8 @@
 #ifndef BITCOIN_SCRIPT_SCRIPT_ERROR_H
 #define BITCOIN_SCRIPT_SCRIPT_ERROR_H
 
+#include <string>
+
 typedef enum ScriptError_t
 {
     SCRIPT_ERR_OK = 0,
@@ -73,6 +75,6 @@ typedef enum ScriptError_t
 
 #define SCRIPT_ERR_LAST SCRIPT_ERR_ERROR_COUNT
 
-const char* ScriptErrorString(const ScriptError error);
+std::string ScriptErrorString(const ScriptError error);
 
 #endif // BITCOIN_SCRIPT_SCRIPT_ERROR_H
diff --git a/src/script/standard.cpp b/src/script/standard.cpp
index 7d89a336fb..c90c2c24a0 100644
--- a/src/script/standard.cpp
+++ b/src/script/standard.cpp
@@ -9,6 +9,8 @@
 #include <pubkey.h>
 #include <script/script.h>
 
+#include <string>
+
 typedef std::vector<unsigned char> valtype;
 
 bool fAcceptDatacarrier = DEFAULT_ACCEPT_DATACARRIER;
@@ -25,7 +27,7 @@ WitnessV0ScriptHash::WitnessV0ScriptHash(const CScript& in)
     CSHA256().Write(in.data(), in.size()).Finalize(begin());
 }
 
-const char* GetTxnOutputType(txnouttype t)
+std::string GetTxnOutputType(txnouttype t)
 {
     switch (t)
     {
@@ -39,7 +41,7 @@ const char* GetTxnOutputType(txnouttype t)
     case TX_WITNESS_V0_SCRIPTHASH: return "witness_v0_scripthash";
     case TX_WITNESS_UNKNOWN: return "witness_unknown";
     }
-    return nullptr;
+    assert(false);
 }
 
 static bool MatchPayToPubkey(const CScript& script, valtype& pubkey)
diff --git a/src/script/standard.h b/src/script/standard.h
index 8e50e064bb..2929425670 100644
--- a/src/script/standard.h
+++ b/src/script/standard.h
@@ -11,6 +11,8 @@
 
 #include <boost/variant.hpp>
 
+#include <string>
+
 
 static const bool DEFAULT_ACCEPT_DATACARRIER = true;
 
@@ -145,7 +147,7 @@ typedef boost::variant<CNoDestination, PKHash, ScriptHash, WitnessV0ScriptHash,
 bool IsValidDestination(const CTxDestination& dest);
 
 /** Get the name of a txnouttype as a C string, or nullptr if unknown. */
-const char* GetTxnOutputType(txnouttype t);
+std::string GetTxnOutputType(txnouttype t);
 
 /**
  * Parse a scriptPubKey and identify script type for standard scripts. If
diff --git a/src/test/script_tests.cpp b/src/test/script_tests.cpp
index 56454f61f3..cb3ae290d1 100644
--- a/src/test/script_tests.cpp
+++ b/src/test/script_tests.cpp
@@ -102,7 +102,7 @@ static ScriptErrorDesc script_errors[]={
     {SCRIPT_ERR_SIG_FINDANDDELETE, "SIG_FINDANDDELETE"},
 };
 
-static const char *FormatScriptError(ScriptError_t err)
+static std::string FormatScriptError(ScriptError_t err)
 {
     for (unsigned int i=0; i<ARRAYLEN(script_errors); ++i)
         if (script_errors[i].err == err)
@@ -134,7 +134,7 @@ void DoTest(const CScript& scriptPubKey, const CScript& scriptSig, const CScript
     CMutableTransaction tx = BuildSpendingTransaction(scriptSig, scriptWitness, txCredit);
     CMutableTransaction tx2 = tx;
     BOOST_CHECK_MESSAGE(VerifyScript(scriptSig, scriptPubKey, &scriptWitness, flags, MutableTransactionSignatureChecker(&tx, 0, txCredit.vout[0].nValue), &err) == expect, message);
-    BOOST_CHECK_MESSAGE(err == scriptError, std::string(FormatScriptError(err)) + " where " + std::string(FormatScriptError((ScriptError_t)scriptError)) + " expected: " + message);
+    BOOST_CHECK_MESSAGE(err == scriptError, FormatScriptError(err) + " where " + FormatScriptError((ScriptError_t)scriptError) + " expected: " + message);
 
     // Verify that removing flags from a passing test or adding flags to a failing test does not change the result.
     for (int i = 0; i < 16; ++i) {