Repository: bitcoin/bitcoin
File: src/uint256.cpp (C++)

Code before:
// Copyright (c) 2009-2020 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 <uint256.h>

#include <util/strencodings.h>

template <unsigned int BITS>
std::string base_blob<BITS>::GetHex() const
{
    uint8_t m_data_rev[WIDTH];
    for (int i = 0; i < WIDTH; ++i) {
        m_data_rev[i] = m_data[WIDTH - 1 - i];
    }
    return HexStr(m_data_rev);
}

template <unsigned int BITS>
void base_blob<BITS>::SetHex(const char* psz)
{
    std::fill(m_data.begin(), m_data.end(), 0);

    // skip leading spaces
    while (IsSpace(*psz))
        psz++;

    // skip 0x
    if (psz[0] == '0' && ToLower(psz[1]) == 'x')
        psz += 2;

    // hex string to uint
    size_t digits = 0;
    while (::HexDigit(psz[digits]) != -1)
        digits++;
    unsigned char* p1 = m_data.data();
    unsigned char* pend = p1 + WIDTH;
    while (digits > 0 && p1 < pend) {
        *p1 = ::HexDigit(psz[--digits]);
        if (digits > 0) {
            *p1 |= ((unsigned char)::HexDigit(psz[--digits]) << 4);
            p1++;
        }
    }
}

template <unsigned int BITS>
void base_blob<BITS>::SetHex(const std::string& str)
{
    SetHex(str.c_str());
}

Review comment (bug):
Not sure about changing code that will be obsolete in a few commits. It seems wasteful to waste review on code that will be changed or deleted down the line. It seems better to touch every line of code only once (or the least number of times possible), than to repeatedly refactor it, and then remove the "feature".

Also, this "refactor" (d38bcd39cd98dd45c8b530f415339cda2422ec9f)  is introducing new bugs and behavior changes, so it seems like a regression either way.

For example, by removing the `std::fill(m_data.begin(), m_data.end(), 0);`, two `SetHex` calls now may result in a different value, because the previous value is not zeroed, but leaked into the new value.

(I haven't checked for other bugs, but I am sure one can find them by spending more than a few seconds on this)

Code after:
// Copyright (c) 2009-2020 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 <uint256.h>

#include <util/strencodings.h>

template <unsigned int BITS>
std::string base_blob<BITS>::GetHex() const
{
    uint8_t m_data_rev[WIDTH];
    for (int i = 0; i < WIDTH; ++i) {
        m_data_rev[i] = m_data[WIDTH - 1 - i];
    }
    return HexStr(m_data_rev);
}

template <unsigned int BITS>
void base_blob<BITS>::SetHex(const std::string_view str)
{
    std::fill(m_data.begin(), m_data.end(), 0);

    const auto trimmed = util::RemovePrefixView(util::TrimStringView(str), "0x");

    // Note: if we are passed a greater number of digits than would fit as bytes
    // in m_data, we will be discarding the leftmost ones.
    // str="12bc" in a WIDTH=1 m_data => m_data[] == "\0xbc", not "0x12".
    size_t digits = 0;
    for (const char c : trimmed) {
        if (::HexDigit(c) == -1) break;
        ++digits;
    }
    unsigned char* p1 = m_data.data();
    unsigned char* pend = p1 + WIDTH;
    while (digits > 0 && p1 < pend) {
        *p1 = ::HexDigit(trimmed[--digits]);
        if (digits > 0) {
            *p1 |= ((unsigned char)::HexDigit(trimmed[--digits]) << 4);
            p1++;
        }
    }
}

template <unsigned int BITS>
std::string base_blob<BITS>::ToString() const
{
    return (GetHex());
}

// Explicit instantiations for base_blob<160>