Explicitly initialize prevector _union by Empact · Pull Request #14028 · bitcoin/bitcoin

Empact · 2018-08-23T09:24:31Z

No description provided.

Empact · 2018-08-23T09:24:41Z

Prompted by #13902

Empact · 2018-08-23T09:42:00Z

src/prevector.h

Compare with:

bitcoin/src/prevector.h

Lines 185 to 192 in 271b379

char* new_indirect = static_cast<char*>(malloc(((size_t)sizeof(T)) * new_capacity));

assert(new_indirect);

T* src = direct_ptr(0);

T* dst = reinterpret_cast<T*>(new_indirect);

memcpy(dst, src, size() * sizeof(T));

_union.indirect = new_indirect;

_union.capacity = new_capacity;

_size += N + 1;

DrahtBot · 2018-08-23T11:29:58Z

No more conflicts as of last run.

sipa · 2018-08-23T16:15:01Z

src/prevector.h

This is wrong. The _size field does not directly encode the size (see the comments above the class).

init_union adds the N + 1 when appropriate.

Fair enough, moved it out of the initializer.

practicalswift · 2018-08-23T17:48:00Z

Concept ACK

This fixes the compiler warning described in #13902

Empact · 2018-08-24T08:53:38Z

Pared the PR down to just explicitly initializing the _union

practicalswift · 2018-08-24T09:14:18Z

The updated version still fixes the compiler warning.

ACK f955a50384ad53e6d1e89f373905ea8dabc46e93

Empact · 2018-08-27T16:51:05Z

Rebased for #14030

practicalswift · 2018-08-27T16:57:21Z

utACK 1d9aa00

ken2812221 · 2018-08-27T17:00:10Z

utACK 1d9aa00

1d9aa00 Explicitly initialize prevector _union (Ben Woosley) Pull request description: Tree-SHA512: 3037a5d63b840a4cb0c3c26593ce1b7e1a6ba273a4ee5072563b20169be9783dbdfe3a38c9651d73b2d18ed9668deaf65f994eca7f225c70f875716f05eda3a6

…laration d2eee87 Lift prevector default vals to the member declaration (Ben Woosley) Pull request description: I overlooked this possibility in #14028 ACKs for commit d2eee8: promag: utACK d2eee87, change looks good because members are always initialized. 251Labs: utACK d2eee87 nice one. ken2812221: utACK d2eee87 practicalswift: utACK d2eee87 scravy: utACK d2eee87 Tree-SHA512: f2726bae1cf892fd680cf8571027bcdc2e42ba567eaa901fb5fb5423b4d11b29e745e0163d82cb513d8c81399cc85933a16ed66d4a30829382d4721ffc41dc97

…ber declaration d2eee87 Lift prevector default vals to the member declaration (Ben Woosley) Pull request description: I overlooked this possibility in bitcoin#14028 ACKs for commit d2eee8: promag: utACK d2eee87, change looks good because members are always initialized. 251Labs: utACK d2eee87 nice one. ken2812221: utACK d2eee87 practicalswift: utACK d2eee87 scravy: utACK d2eee87 Tree-SHA512: f2726bae1cf892fd680cf8571027bcdc2e42ba567eaa901fb5fb5423b4d11b29e745e0163d82cb513d8c81399cc85933a16ed66d4a30829382d4721ffc41dc97

a8b965e Explicitly initialize prevector _union (random-zebra) bdd98e8 [Trivial] Add a comment on the use of prevector in script. (random-zebra) bbaa6e3 Clarify prevector::erase and avoid swap-to-clear (random-zebra) 28a8435 prevector: assert successful allocation (random-zebra) de41cb5 Only call clear on prevector if it isn't trivially destructible (random-zebra) 8784df3 Make CScript (and prevector) c++11 movable. (random-zebra) 83f9ac6 serialize: Deprecate `begin_ptr` / `end_ptr` (random-zebra) c3ecc12 prevector: add C++11-like data() method (random-zebra) 5490aa0 test prevector::swap (random-zebra) 035760e prevector::swap: fix (unreached) data corruption (random-zebra) 0e71400 prevector: destroy elements only via erase() (random-zebra) 9811a68 [Core] Prevector type (random-zebra) Pull request description: Based on: - [x] #1554 This introduces `prevector<N, T>`, a new basic data type which is a fully API compatible drop-in replacement for `std::vector<T>` and uses it for the script, to reduce the considerable memory overhead of vectors in cases where they normally contain a small number of small elements. Original tests in Bitcoin showed a reduction in dbcache memory usage by **23%**, and made an initial sync **13%** faster. Backported from the following upstream's PRs, with only additional edits in the 2nd layer scripts (`masternode-payments`, `masternode-budget`) and in the unit tests (to address the differences in `insecure_rand`): - bitcoin#6914 - bitcoin#7888 - bitcoin#8850 - bitcoin#9349 - bitcoin#9505 - bitcoin#9856 - bitcoin#10534 - bitcoin#11011 - bitcoin#14028 ~~NOTE: Updates to `memusage.h` needed as well, when #1531 is merged.~~ ACKs for top commit: furszy: Tested ACK a8b965e . Fuzzbawls: ACK a8b965e Tree-SHA512: 203b660dd8d9f98780be660dae0ac951766ba438836bd6f0ec921e7749c1a84143f3b920fe49f2cc399f4aa7e763098f78746d3b6ff845bcf913bb13de984bc1

1d9aa00 Explicitly initialize prevector _union (Ben Woosley) Pull request description: Tree-SHA512: 3037a5d63b840a4cb0c3c26593ce1b7e1a6ba273a4ee5072563b20169be9783dbdfe3a38c9651d73b2d18ed9668deaf65f994eca7f225c70f875716f05eda3a6

…ber declaration d2eee87 Lift prevector default vals to the member declaration (Ben Woosley) Pull request description: I overlooked this possibility in bitcoin#14028 ACKs for commit d2eee8: promag: utACK d2eee87, change looks good because members are always initialized. 251Labs: utACK d2eee87 nice one. ken2812221: utACK d2eee87 practicalswift: utACK d2eee87 scravy: utACK d2eee87 Tree-SHA512: f2726bae1cf892fd680cf8571027bcdc2e42ba567eaa901fb5fb5423b4d11b29e745e0163d82cb513d8c81399cc85933a16ed66d4a30829382d4721ffc41dc97

fanquake added the Refactoring label Aug 23, 2018

Empact mentioned this pull request Aug 23, 2018

prevector: Suppress two -Wmaybe-uninitialized (gcc) warnings in indirect_ptr(...) #13902

Closed

Empact force-pushed the prevector-explicit-initialization branch 2 times, most recently from eeea910 to bfa1d03 Compare August 23, 2018 09:39

Empact commented Aug 23, 2018

View reviewed changes

DrahtBot mentioned this pull request Aug 23, 2018

Remove ambiguity in construction of prevector #14030

Merged

sipa reviewed Aug 23, 2018

View reviewed changes

Empact force-pushed the prevector-explicit-initialization branch 4 times, most recently from 2e2b691 to b8212af Compare August 23, 2018 17:31

Empact force-pushed the prevector-explicit-initialization branch 2 times, most recently from 63dc8f2 to f955a50 Compare August 24, 2018 08:53

DrahtBot added the Needs rebase label Aug 27, 2018

Explicitly initialize prevector _union

1d9aa00

Empact force-pushed the prevector-explicit-initialization branch from f955a50 to 1d9aa00 Compare August 27, 2018 16:50

DrahtBot removed the Needs rebase label Aug 27, 2018

laanwj merged commit 1d9aa00 into bitcoin:master Aug 29, 2018

Bushstar mentioned this pull request Sep 4, 2018

commits from bitcoin/master FeatherCoin/Feathercoin#359

Merged

Empact mentioned this pull request Sep 19, 2018

refactor: Lift prevector default vals to the member declaration #14266

Merged

Empact deleted the prevector-explicit-initialization branch October 9, 2018 05:23

cmihai mentioned this pull request May 2, 2019

Fix GCC warnings in prevector.h by initializing class fields dtr-org/unit-e#1047

Merged

sickpig mentioned this pull request Mar 2, 2020

[port] update prevector.h BitcoinUnlimited/BitcoinUnlimited#2109

Merged

random-zebra mentioned this pull request Apr 24, 2020

[Core] Prevector PIVX-Project/PIVX#1557

Merged

1 task

bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021

	char* new_indirect = static_cast<char>(malloc(((size_t)sizeof(T)) new_capacity));
	assert(new_indirect);
	T* src = direct_ptr(0);
	T* dst = reinterpret_cast<T*>(new_indirect);
	memcpy(dst, src, size() * sizeof(T));
	_union.indirect = new_indirect;
	_union.capacity = new_capacity;
	_size += N + 1;

Conversation

Empact commented Aug 23, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Empact commented Aug 23, 2018

Uh oh!

Empact Aug 23, 2018

Choose a reason for hiding this comment

Uh oh!

DrahtBot commented Aug 23, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sipa Aug 23, 2018

Choose a reason for hiding this comment

Uh oh!

Empact Aug 23, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sipa Aug 23, 2018

Choose a reason for hiding this comment

Uh oh!

Empact Aug 23, 2018

Choose a reason for hiding this comment

Uh oh!

practicalswift commented Aug 23, 2018

Uh oh!

Empact commented Aug 24, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

practicalswift commented Aug 24, 2018

Uh oh!

Empact commented Aug 27, 2018

Uh oh!

practicalswift commented Aug 27, 2018

Uh oh!

ken2812221 commented Aug 27, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Empact commented Aug 23, 2018 •

edited

Loading

DrahtBot commented Aug 23, 2018 •

edited

Loading

Empact Aug 23, 2018 •

edited

Loading

Empact commented Aug 24, 2018 •

edited

Loading