Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix boundary condition in skip method #113

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

defitricks
Copy link

This update addresses a critical boundary condition issue in the skip method of the Source implementation for (Arc<Vec<G>>, usize).

What was the issue?

Previously, the check for sufficient elements in the vector was incorrectly written as:

if self.0.len() <= self.1 {

This condition only ensured that the current index self.1 was within bounds but did not account for the additional amt elements being skipped. As a result, if amt was large enough, the resulting index (self.1 + amt) could exceed the vector's length, leading to undefined behavior or runtime panics when accessing elements out of bounds.

What has been changed?

The condition has been updated to:

if self.0.len() < self.1 + amt {

This guarantees that the entire range [self.1, self.1 + amt) lies within the bounds of the vector, preventing invalid memory access.

Why is this important?

  1. Safety: Prevents potential crashes or undefined behavior when processing sources with a large amt in the skip method.
  2. Correctness: Ensures that the logic adheres to the expected behavior of skipping exactly amt elements, only if they exist.
  3. Robustness: Improves reliability when dealing with large datasets or when operating in multithreaded contexts where boundaries are critical.

This change is critical for anyone relying on this functionality, especially in production scenarios involving complex computations or parallelism.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant