Small bug fixes and cleanups from fuzz testing #3299
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
switch to acceptSpaces by default
greglandrum
added
bug
Cleanup
bp-kelley
reviewed
Jul 21, 2020
| @@ -145,28 +145,56 @@ inline boost::uint32_t readPackedIntFromStream(std::stringstream &ss) { | |||
| int shift, offset; | |||
| char tmp; | |||
| ss.read(&tmp, sizeof(tmp)); | |||
| if (ss.fail()) { | |||
There was a problem hiding this comment.
Perhaps we could roll this into an ss_check inline function. We might want to check for premature eof as well.
There was a problem hiding this comment.
I thought about that, but a premature EOF in the middle of reading the char here should result in the failbit being set (since the requested number of bytes wasn't read), so that case is covered. And if we hit EOF after reading the bytes we don't care (at least not here).
|
Except for the comment, all looks good. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR includes a number of small fixes and changes to problems that showed up during the fuzz testing:
toInt()andtoDouble()functions used during the MOL file parsing now generate errors if the input strings contain characters that cannot appear in numbers. This is necessary because bothstrtolandatofhappily return zero if they fail. Switching toboost::lexical_castwould be less code, but it is significantly slower. The changes here do not noticeably impact the speed of the code (I benchmarked mol file parsing to be sure of this)M MRV SMAis now stricter. We may want to consider extending this strictness to other CTAB tags.makeAHAtomQuery()was incorrectly returning anATOM_EQUALS_QUERY *. That query is anATOM_NULL_QUERY, which is different.StreamOps.hnow explicitly throw anstd::runtime_error()if they fail to read the required data from the stream.