refactor: Remove ParseHex(const char*) from public interface #24751

This is a refactor (with added test), needed for:

• #23595
• #18061
• Some bugfixes I am working on, but are blocked on this

Code Review ACK fad1b614b55e0373ba73949f30a3bb360f6db25a

• This PR:
  1. Add test for ParseHex function. Specifically, it adds tests for an embedded null (\0).
  2. Removes a redundant version of the ParseHex function cleaning the code.

These two commits are separated from #23595. While both the commits are refactoring changes, the changes done in the second commit are commonly shared between the PRs #23595 and #18061.

I agree with separating these two commits in a separate PR because:

1. It removes the need of repeated efforts on a similar change in two different PRs.
2. It separates refactoring changes from other subsequent changes done in the PRs.
3. It makes the PRs more focused on a specific change, which aligns with the philosophy of atomic transitions.

The updated code is clean and easy to understand and reason with, and I think it can be merged.

Concept ACK on getting rid of const char * string interfaces.

Code review ACK fad1b614b55e0373ba73949f30a3bb360f6db25a

ACK https://github.com/bitcoin/bitcoin/pull/24751/commits/fad1b614b55e0373ba73949f30a3bb360f6db25a

Using std::string_view would be ideal here, as constructing one of those from either std::string or const char* is efficient, and without allocation overhead.

@sipa This is ideal, but non-trivial IIUC. See the previous code discussion.

diff --git a/src/util/strencodings.cpp b/src/util/strencodings.cpp
index 940fa90da2..691607caba 100644
--- a/src/util/strencodings.cpp
+++ b/src/util/strencodings.cpp
@@ -81,32 +81,24 @@ bool IsHexNumber(const std::string& str)
     return (str.size() > starting_location);
 }
 
-std::vector<unsigned char> ParseHex(const char* psz)
+std::vector<unsigned char> ParseHex(std::string_view str)
 {
     // convert hex dump to vector
     std::vector<unsigned char> vch;
-    while (true)
-    {
-        while (IsSpace(*psz))
-            psz++;
-        signed char c = HexDigit(*psz++);
-        if (c == (signed char)-1)
-            break;
-        auto n{uint8_t(c << 4)};
-        c = HexDigit(*psz++);
-        if (c == (signed char)-1)
-            break;
-        n |= c;
-        vch.push_back(n);
+    auto it = str.begin();
+    while (it != str.end() && it + 1 != str.end()) {
+        if (IsSpace(*it)) {
+            ++it;
+            continue;
+        }
+        auto c1 = HexDigit(*(it++));
+        auto c2 = HexDigit(*(it++));
+        if (c1 == (signed char)-1 || c2 == (signed char)-1) break;
+        vch.push_back(uint8_t(c1 << 4) | c2);
     }
     return vch;
 }
 
-std::vector<unsigned char> ParseHex(const std::string& str)
-{
-    return ParseHex(str.c_str());
-}
-
 void SplitHostPort(std::string in, uint16_t& portOut, std::string& hostOut)
 {
     size_t colon = in.find_last_of(':');
diff --git a/src/util/strencodings.h b/src/util/strencodings.h
index 1f83fa3ffa..0e72466fc3 100644
--- a/src/util/strencodings.h
+++ b/src/util/strencodings.h
@@ -55,8 +55,7 @@ enum class ByteUnit : uint64_t {
 * [@return](/bitcoin-bitcoin/contributor/return/)           A new string without unsafe chars
 */
 std::string SanitizeString(const std::string& str, int rule = SAFE_CHARS_DEFAULT);
-std::vector<unsigned char> ParseHex(const char* psz);
-std::vector<unsigned char> ParseHex(const std::string& str);
+std::vector<unsigned char> ParseHex(std::string_view str);
 signed char HexDigit(char c);
 /* Returns true if each character in str is a hex character, and has an even
  * number of hex digits.*/

?

Happy to review, if you create a pull. With "trivial" I meant that this is not a one-line fix, but requires reworking the algorithm.

On it.

See #24764.

<!--e57a25ab6845829454e8d69fc972939a-->

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

<!--174a7506f384e20aa4161008e828411d-->

Conflicts

Reviewers, this pull request conflicts with the following ones:

• #24764 (Modernize util/strencodings and util/string: string_view and optional by sipa)
• #22087 (Validate port-options by amadeuszpawlik)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

Thanks, closing for now.