Use saturating arithmetic to avoid usize overflow panics (#1066, #1067, #1068)#1109
Open
patrickwehbe wants to merge 1 commit into
Open
Use saturating arithmetic to avoid usize overflow panics (#1066, #1067, #1068)#1109patrickwehbe wants to merge 1 commit into
patrickwehbe wants to merge 1 commit into
Conversation
Three methods could panic with an attempt-to-add/multiply-with-overflow when given very large inputs, in debug builds: - k_smallest_relaxed allocated room for `2 * k` items, which overflowed for k greater than usize::MAX / 2. Use saturating_mul so it saturates to usize::MAX and collects the whole iterator instead. - peek_nth and peek_nth_mut computed `n + 1`, which overflowed for n == usize::MAX. Use saturating_add so peeking that far just returns None, matching the behaviour of peeking past the end. - InterleaveShortest::size_hint computed `combined_lower + 1` for the lower bound, which overflowed when the combined lower bound was usize::MAX. Use saturating_add. The upper-bound branch right below already used checked_add, so this lines the two up. Added regression tests for each in tests/test_std.rs. Fixes rust-itertools#1066 Fixes rust-itertools#1067 Fixes rust-itertools#1068
This file contains hidden or 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
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.
Three
Itertoolsmethods can panic with an overflow in debug builds whengiven very large inputs. Each one does an unchecked
+ 1or* 2on ausizethat can already be near the maximum. This switches them tosaturating arithmetic.
k_smallest_relaxed(issue Bug ink_smallest_relaxed? #1066): it allocates room for2 * kitems,which overflows once
kis greater thanusize::MAX / 2. Now it usessaturating_mul, so a hugeksaturates tousize::MAXand the wholeiterator is collected, the same result you already get from
k_smallest.peek_nthandpeek_nth_mut(issue Peekingusize::MAXwithpeek_nth#1067): both computen + 1, whichoverflows when
n == usize::MAX. Now they usesaturating_add, sopeeking that far just returns
None, which is what peeking past the endalready does.
InterleaveShortest::size_hint(issueinterleave_shortest's size_hint panics with large iterators. #1068): the lower bound is computedas
combined_lower + 1, which overflows whencombined_lowerisusize::MAX. Now it usessaturating_add. The upper-bound branch a fewlines down already uses
checked_add, so this just brings the two intoline.
I added a regression test for each in
tests/test_std.rsthat calls theaffected method with
usize::MAX-ish input and checks it returns instead ofpanicking.
The reporter (@Takashiidobe) suggested the saturating approach in #1066;
the same idea applies cleanly to all three.
Fixes #1066
Fixes #1067
Fixes #1068