Use ReadLE64 in uint256::GetUint64 instead of duplicating logic #26105

pull sipa wants to merge 1 commits into bitcoin:master from sipa:202209_uint256_readle64 changing 1 files +2 −9
  1. sipa commented at 8:50 PM on September 15, 2022: member

    No need to have a (naive) copy of the ReadLE64 logic inside uint256::GetUint64, when we have an optimized function for exactly that.

  2. Use ReadLE64 in uint256::GetUint64() instead of duplicating logic 04fee75bac
  3. davidgumberg commented at 5:20 AM on September 16, 2022: contributor

    ACK 04fee75bacb9ec3bceff1246ba6c8ed8a8759548

    The existing code assumes host endianness.

  4. jonatack commented at 9:14 AM on September 16, 2022: contributor

    ACK 04fee75bacb9ec3bceff1246ba6c8ed8a8759548 review, this use of ReadLE64() is similar to the existing invocation by Num3072::Num3072(), sanity checked that before and after this change GetUint64() returns the same result (debug build, clang 13)

    --- a/src/uint256.h
    +++ b/src/uint256.h
    @@ -7,6 +7,7 @@
     #include <crypto/common.h>
    +#include <logging.h>
     #include <span.h>
     
    @@ -85,7 +86,20 @@ public:
     
         uint64_t GetUint64(int pos) const
         {
    -        return ReadLE64(m_data + pos * 8);
    +        const uint8_t* ptr = m_data + pos * 8;
    +        uint64_t x = ((uint64_t)ptr[0]) | \
    +                     ((uint64_t)ptr[1]) << 8 | \
    +                     ((uint64_t)ptr[2]) << 16 | \
    +                     ((uint64_t)ptr[3]) << 24 | \
    +                     ((uint64_t)ptr[4]) << 32 | \
    +                     ((uint64_t)ptr[5]) << 40 | \
    +                     ((uint64_t)ptr[6]) << 48 | \
    +                     ((uint64_t)ptr[7]) << 56;
    +        LogPrintf("GetUint64 old: %s\n", x);
    +
    +        const auto y = ReadLE64(m_data + pos * 8);
    +        LogPrintf("GetUint64 new: %s\n", y);
    +
    +        assert(x == y);
    +        return y;
         }
    
    2022-09-16T09:13:53Z GetUint64 old: 2830827502690553650
    2022-09-16T09:13:53Z GetUint64 new: 2830827502690553650
    2022-09-16T09:13:53Z GetUint64 old: 13427380872610120290
    2022-09-16T09:13:53Z GetUint64 new: 13427380872610120290
    2022-09-16T09:13:53Z GetUint64 old: 12183277635375193944
    2022-09-16T09:13:53Z GetUint64 new: 12183277635375193944
    2022-09-16T09:13:53Z GetUint64 old: 16580367070613284548
    2022-09-16T09:13:53Z GetUint64 new: 16580367070613284548
    2022-09-16T09:13:53Z GetUint64 old: 16949092074016075712
    2022-09-16T09:13:53Z GetUint64 new: 16949092074016075712
    2022-09-16T09:13:53Z GetUint64 old: 14891467430351852074
    2022-09-16T09:13:53Z GetUint64 new: 14891467430351852074
    2022-09-16T09:13:53Z GetUint64 old: 2319828850847527094
    2022-09-16T09:13:53Z GetUint64 new: 2319828850847527094
    2022-09-16T09:13:53Z GetUint64 old: 921024485749193602
    2022-09-16T09:13:53Z GetUint64 new: 921024485749193602
    
  5. fanquake merged this on Sep 16, 2022
  6. fanquake closed this on Sep 16, 2022

  7. sipa commented at 12:16 PM on September 16, 2022: member

    The existing code assumes host endianness.

    No, it does not. Both the old and new code interpret the bytes as little-endian.

  8. davidgumberg commented at 12:25 PM on September 16, 2022: contributor

    The existing code assumes host endianness.

    No, it does not. Both the old and new code interpret the bytes as little-endian.

    Right, I meant that the old code interprets the bytes as little endian and stores them as little endian, even if the host machine is big-endian. ReadLE64 invokes le64toh which does not make an assumption about "host" endianness.

  9. sidhujag referenced this in commit 2cdf7113f2 on Sep 20, 2022
  10. bitcoin locked this on Sep 16, 2023

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-04-19 09:13 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me