Skip to content

Conversation

@pstratem
Copy link
Contributor

@pstratem pstratem commented Apr 25, 2016

Previously disk corruption would cause an assert instead of an exception.

This has not been extensively reviewed.

@pstratem pstratem force-pushed the 2016-04-24-cdatastream-ignore branch from d3c728a to 2ed0ca4 Compare April 25, 2016 07:34
Previously disk corruption would cause an assert instead of an exception.
@laanwj
Copy link
Member

laanwj commented Apr 25, 2016

Concept ACK.

How do the call-sites of this function cope with this exception? I suppose this is only used in reindexing?

@sipa
Copy link
Member

sipa commented Apr 25, 2016

There are actually no call sites for this before #7933...

@pstratem
Copy link
Contributor Author

CDataStream::ignore -> CScriptCompressor::Unserialize -> CTxOutCompressor::SerializationOp

CCoins::GetSerializeSize,Serialize,Unserialize
CTxInUndo::GetSerializeSize,Serialize,Unserialize

Still working on what calls those...

@pstratem pstratem changed the title CDataStream::ignore Throw exception instead of assert on negative nSize. [WIP] CDataStream::ignore Throw exception instead of assert on negative nSize. Apr 25, 2016
@pstratem pstratem changed the title [WIP] CDataStream::ignore Throw exception instead of assert on negative nSize. CDataStream::ignore Throw exception instead of assert on negative nSize. Apr 25, 2016
@sipa
Copy link
Member

sipa commented Apr 25, 2016

@pstratem I believe it is. I'm going to include it in #7933, as without it, the test I'm adding for it fails...

@pstratem
Copy link
Contributor Author

@sipa Indeed since this can only be triggered by disk corruption... all bets are off anyways

@laanwj
Copy link
Member

laanwj commented Apr 26, 2016

Merged as part of #7933

@laanwj laanwj closed this Apr 26, 2016
@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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants