uint256: 1 is a constant #20016
uint256: 1 is a constant #20016
Conversation
|
This was added in #17261 in which it was remarked "static initialization order result[ed] in a potential uninitialized object access." I have an alternative change at https://github.com/ajtowns/bitcoin/commits/202009-uint256_1_v2 which does a static init and hence avoids the functional call, and seems to pass travis fine, so might be worth considering that? cc @achow101 @kallewoof |
Concept ACK |
|
Clear improvement. ACK 832109a |
832109a to
915c1c2
Compare
|
Updated to simplfy constructors and replace |
|
Concept ACK |
|
utACK 915c1c2
It is in tautologies that the ordering of society is centered. |
There was a problem hiding this comment.
Not sure how this could have led to memory access violations. Though, if the wallet or the script interpreter is running after the end of the main function, something else has gone wrong and should probably be fixed.
cr ACK 915c1c2 📺
Show signature and timestamp
Signature:
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512
cr ACK 915c1c25fd6d6bdc249b71be12db0744312729da 📺
-----BEGIN PGP SIGNATURE-----
iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
pUhQlQwAqTJdOujLC6CD+i1XI8ISvOTG76CLhzgK5wg+63FYUVILodO1T0ig7NJH
D+RHnv0fdyO3OIFGH3C4ooSkHNqIiKec7Q0V2uemtU5TZX04jM6ocsP/frsCfSmU
p13UkuNxx5QSnrSlFLxrGrmuX/BkLxdubGLTXNQHNltx5rHz2p0H03ueep2/XrXM
BeMAdrBcBvjsQ3SW5XuUND8hza6YajWpAMOUSnS7rbT/hpxi3SakjYyA8E/fMQUo
tr7F/XVLX9KTYzRFvbQLRdUs2Uy3jIP4AhuJJqa7rt/1wv3Au7N0nL96wPJlBiKO
radqu65JIp2sNDl4XtXcq1gol+O184rASiaeDZlI1kAxUKoBvDKd9XtBnDWHywyl
vvulJWWkZLHN6q0z3JJ2GON8Uyr+q6ErUrOuOyLsuRvnrmawqkc6tBaUpkCefPuQ
6kKwWybrM5xSq0/OYiOQ0vFRJY34GLy7qNmyUOIQrGpbfn9AL7c9B5p9FueqMukP
/HlO1n43
=ZuQH
-----END PGP SIGNATURE-----
Timestamp of file with hash ed0494b57cec88e0ff8da75d55c543250f502605792f0e336b3c5d14e6a470b4 -
|
(1 is not a constant in python, but we don't use that here https://kate.io/blog/2017/08/22/weird-python-integers/) |
There was a problem hiding this comment.
ACK 915c1c2, I have reviewed the code and it looks OK, I agree it can be merged.
May I suggest a further step:
--- a/src/uint256.h
+++ b/src/uint256.h
@@ -21,7 +21,7 @@ protected:
uint8_t m_data[WIDTH];
public:
/* construct 0 value by default */
- base_blob() : m_data() {}
+ constexpr base_blob() : m_data() {}
/* constructor for constants between 1 and 255 */
explicit constexpr base_blob(uint8_t v) : m_data{v} {}
@@ -112,7 +112,7 @@ public:
*/
class uint160 : public base_blob<160> {
public:
- uint160() {}
+ constexpr uint160() {}
explicit uint160(const std::vector<unsigned char>& vch) : base_blob<160>(vch) {}
};
@@ -123,7 +123,7 @@ public:
*/
class uint256 : public base_blob<256> {
public:
- uint256() {}
+ constexpr uint256() {}
constexpr explicit uint256(uint8_t v) : base_blob<256>(v) {}
explicit uint256(const std::vector<unsigned char>& vch) : base_blob<256>(vch) {}
static const uint256 ONE;?
Replace the memset with C++11 value/aggregate initialisation of the m_data array, which still ensures the unspecified values end up as zero-initialised. This then allows changing UINT256_ONE() from dynamically allocating an object, to a simpler referencing a static allocation.
-BEGIN VERIFY SCRIPT-
sed -i '/inline.* UINT256_ONE() {/,+1d' src/uint256.h
sed -i 's/UINT256_ONE()/uint256::ONE/' $(git grep -l UINT256_ONE)
-END VERIFY SCRIPT-
915c1c2 to
4cc7171
Compare
|
ACK 4cc7171 |
|
re ACK 4cc7171, only change is some constexpr shuffling 🛁 Show signature and timestampSignature: Timestamp of file with hash |
Summary: 4cc7171c9887e88027642927c979e507d7b78dda wallet: no need for duplicate storage for ABANDON_HASH constant (Anthony Towns) 82cf4641f4a161834d07ce83c18982d9b143c040 scripted-diff: Replace UINT256_ONE() with uint256::ONE (Anthony Towns) 183f308fff4caad3e3ada654b6fdf597d262c4c1 uint256: Update constructors to c++11, make ONE static (Anthony Towns) Pull request description: `UINT256_ONE()` returns a reference to a global; mark it as const to be sure someone doesn't accidently modify it. --- Backport of [[bitcoin/bitcoin#20016 | core#20016]] Test Plan: ninja all check check-functional Reviewers: #bitcoin_abc, deadalnix Reviewed By: #bitcoin_abc, deadalnix Differential Revision: https://reviews.bitcoinabc.org/D9477
UINT256_ONE()returns a reference to a global; mark it as const to be sure someone doesn't accidently modify it.