Merge #9804: Fixes subscript 0 (&var[0]) where should use (var.data()) instead.
ClosedPublic
Actions

Authored by nakihito on May 7 2019, 03:45.

Details

Reviewers

deadalnix
Fabien
jasonbcox

Group Reviewers

Restricted Owners Package	(Owns No Changed Paths)
Restricted Project

Commits

rSTAGING24ad36f71b92: Merge #9804: Fixes subscript 0 (&var[0]) where should use (var.data()) instead.
rABC24ad36f71b92: Merge #9804: Fixes subscript 0 (&var[0]) where should use (var.data()) instead.

Summary

30ac7688e Fix subscript[0] potential bugs in key.cpp (Jeremy Rubin)
4b1c0f2e2 Remove unnecessary branches in utilstrencodings string constructors. (Jeremy Rubin)
e19db7b5a Fix subscript[0] in utilstrencodings.cpp (Jeremy Rubin)
bc2e7fd98 Fix subscript[0] in streams.h (Jeremy Rubin)
4cac0d1e0 Fix subscript[0] in validation.cpp (Jeremy Rubin)
ac658e55f Fix subscript[0] in torcontrol (Jeremy Rubin)
b6856ebed Fix subscript[0] in netaddress.cpp (Jeremy Rubin)
361d95265 Fix subscript[0] in base58.cpp (Jeremy Rubin)
6896dbf16 Cleanup (safe, it was checked) subscript[0] in MurmurHash3 (and cleanup MurmurHash3 to be more clear). (Jeremy Rubin)
96f2119e6 Fix subscript[0] in compressor.cpp (Jeremy Rubin)
500710bd2 Fix 2 subscript[0] bugs in pubkey.cpp, and eliminate one extra size check (Jeremy Rubin)
e0451e3e2 Fix subscript[0] bug in net.cpp if GetGroup returns a 0-sized vector (Jeremy Rubin)

Tree-SHA512: 5b9103652cf8c615bd8f4f32b3573d291d6b67c39e0308ce00100bc6625f346e8e016b4c999f4f34f5c37ae059490a83c3b513deb21f838af785227d06e02362

Backport of Core PR 9693
https://github.com/bitcoin/bitcoin/pull/9804/

Test Plan

make check
test_runner.py

Diff Detail

Repository

rABC Bitcoin ABC

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

nakihito created this revision.May 7 2019, 03:45

Owners added a reviewer: Restricted Owners Package.May 7 2019, 03:45

Herald added a reviewer: Restricted Project. · View Herald TranscriptMay 7 2019, 03:45

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing started.

nakihito planned changes to this revision.May 7 2019, 03:54

nakihito added inline comments.

src/pubkey.cpp
325 ↗	(On Diff #8452)	This part here is supposed to be: if (!ecdsa_signature_parse_der_lax(secp256k1_context_verify, &sig, vchSig.data(), vchSig.size())) { However, this causes a compiler error stating `.data()` is not a member function. I'm not sure how to rectify this.

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing passed.

Harbormaster completed remote builds in B5723: Diff 8452.May 7 2019, 04:00

nakihito requested review of this revision.May 7 2019, 19:58

nakihito added inline comments.

src/pubkey.cpp
325 ↗	(On Diff #8452)	There does not appear to be an equivalent .data() function in boost::sliced_range (at least that I could find). As such, this will have to stay unchanged.

markblundeberg added a subscriber: markblundeberg.May 7 2019, 23:17

markblundeberg added inline comments.

src/pubkey.cpp
325 ↗	(On Diff #8452)	sliced_range doesn't appear anywhere in Core and is an ABC thing from D1572 :/

Fabien added inline comments.May 9 2019, 07:39

src/pubkey.cpp
325 ↗	(On Diff #8452)	What about `&vchSig.front()` ?

Previously mentioned issue was resolved with Fabien's suggestion of using .front().

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing started.

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing passed.

Harbormaster completed remote builds in B5809: Diff 8604.May 10 2019, 18:20

Fabien requested changes to this revision.May 13 2019, 12:44

Fabien added inline comments.

src/hash.cpp
43 ↗	(On Diff #8604)	Note to reviewers: while checking this against UB I found this interesting article: http://www.drdobbs.com/cpp/why-does-c-allow-arithmetic-on-null-poin/240001022. Conclusion: this is UB in C but not in C++.
src/streams.h
428 ↗	(On Diff #8604)	Braces