Skip to content

Conversation

TheBlueMatt
Copy link
Contributor

No description provided.

laanwj added 2 commits April 27, 2016 11:11
I made a silly mistake in a database wrapper where keys
were sorted by char instead of uint8_t. As x86 char is signed
the sorting for the block index database was messed up, resulting
in a segfault due to missing records.

Add a test to catch:
- Wrong sorting
- Seeking errors
- Iteration result not complete
@laanwj laanwj added the Tests label May 3, 2016
@laanwj
Copy link
Member

laanwj commented May 3, 2016

Thanks!
(continues #7956)

inline void SerializationOp(Stream& s, Operation ser_action, int nType, int nVersion) {
if (ser_action.ForRead()) {
str.clear();
char c;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please initialize this (as clang said 8):

char c = '\0';

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

@paveljanik
Copy link
Contributor

Concept ACK

@sipa
Copy link
Member

sipa commented May 17, 2016

@TheBlueMatt Can you address 7be4754#r61928804 ?

@sipa sipa mentioned this pull request May 26, 2016
@sipa sipa merged commit 269a440 into bitcoin:master Jun 2, 2016
sipa added a commit that referenced this pull request Jun 2, 2016
269a440 Add test for dbwrapper iterators with same-prefix keys. (Matt Corallo)
6030625 test: Add more thorough test for dbwrapper iterators (Wladimir J. van der Laan)
84c13e7 chain: Add assertion in case of missing records in index db (Wladimir J. van der Laan)
maflcko pushed a commit to maflcko/bitcoin-core that referenced this pull request Jun 9, 2016
zkbot added a commit to zcash/zcash that referenced this pull request Apr 4, 2018
Additional dbwrapper tests

Cherry-picked from the following upstream PRs:

- bitcoin/bitcoin#7992
- bitcoin/bitcoin#9867
  - Only the commit affecting dbwrapper tests
- bitcoin/bitcoin#9610
  - Only the change affecting dbwrapper tests
- bitcoin/bitcoin#10844
random-zebra added a commit to PIVX-Project/PIVX that referenced this pull request Apr 23, 2021
bc3d3fa Add testcase to simulate bitcoin schema in leveldb (MapleLaker)
4f15155 Verify DBWrapper iterators are taking snapshots (Matt Corallo)
36b407e test: Replace remaining sprintf with snprintf (Wladimir J. van der Laan)
e8cb38a Add test for dbwrapper iterators with same-prefix keys. (Matt Corallo)
4a4ddb8 test: Add more thorough test for dbwrapper iterators I made a silly mistake in a database wrapper where keys were sorted by char instead of uint8_t. As x86 char is signed the sorting for the block index database was messed up, resulting in a segfault due to missing records. (Wladimir J. van der Laan)
109446f chain: Add assertion in case of missing records in index db (Wladimir J. van der Laan)

Pull request description:

  Back ported the following PRs from upstream (adaptations were needed because we aren't obfuscating the db):

  * bitcoin#7992.
  * bitcoin#9867.
  * bitcoin#11422.
  * bitcoin#17206

ACKs for top commit:
  random-zebra:
    Nice! ACK bc3d3fa

Tree-SHA512: ebd811d7ee0d970247ceec2624e524df7611274f36ef7c0252712ebf247828ee091dcc6872585271fac3d7846127a997a856d7cd72eb20425e06213207ff1c7a
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants