All char representations are serialized in the same way, however the char one is deprecated according to https://github.com/bitcoin/bitcoin/blob/d22e7ee93313b13365bd14a5fffeb055cff4dcd2/src/serialize.h#L227 . Also, using uint8_t directly avoids casts.
refactor: [index] Replace deprecated char with uint8_t in serialization #21824
pull MarcoFalke wants to merge 1 commits into bitcoin:master from MarcoFalke:2105-refactorUint8_t changing 4 files +20 −21-
MarcoFalke commented at 4:16 PM on May 1, 2021: member
-
refactor: [index] Replace deprecated char with uint8_t in serialization fafb880e88
- DrahtBot added the label Refactoring on May 1, 2021
- DrahtBot added the label UTXO Db and Indexes on May 1, 2021
-
jonatack commented at 7:52 PM on May 1, 2021: member
Nice change.
Approach ACK fafb880e8854f9b7fb3934e02a0bd0409aec72c2
-
kristapsk commented at 10:57 PM on May 1, 2021: contributor
Concept ACK,
charis bad in C++ in general for both it's undefined default signedness and a lot of people thinking it's garanteed to be 8-bit, which is not true.Haven't yet looked though all of the code, but shouldn't
charversions ofSerialize()/Unserialize()insrc/serialize.hbe removed too? -
practicalswift commented at 7:41 PM on May 2, 2021: contributor
Concept ACK
-
laanwj commented at 2:14 PM on May 4, 2021: member
Code review ACK fafb880e8854f9b7fb3934e02a0bd0409aec72c2 Using explicitly signed/unsigned sized types is clearly superior for serialization purposes
-
practicalswift commented at 6:37 PM on May 4, 2021: contributor
cr ACK fafb880e8854f9b7fb3934e02a0bd0409aec72c2: patch looks correct
uint8_tand uniform initialisation{}make things significantly easier to reason about. Thanks! - fanquake merged this on May 5, 2021
- fanquake closed this on May 5, 2021
- MarcoFalke deleted the branch on May 5, 2021
- sidhujag referenced this in commit 24dc4f4dfb on May 5, 2021
- gwillen referenced this in commit 44f58478b0 on Jun 1, 2022
- DrahtBot locked this on Aug 16, 2022
