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

core: advance reader's position if pattern cannot be found, fixes out of memory from #301 #344

Merged
merged 1 commit into from
Jan 18, 2025

Conversation

sscobici
Copy link

This addresses out-of-memory errors mentioned in the following comment: #301 (comment).

Issue description:

For a big buffer where the pattern was not found at the end, the reader advanced its position to the end of the buffer.
For a buffer smaller than the pattern length, the reader did not advance its position and entered into infinite loop while reading tags.

This commit ensures that the reader's position is always advanced, resolving the issue.

return Ok(&self.buf[start..end]);
}

let mut j = start;
let mut i = start + pattern.len();
let mut i = start;
Copy link
Owner

Choose a reason for hiding this comment

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

Why were these renamed? Usually the outer loop variable is i, and j is the inner loop. It adds noise the commit. Or am I missing something subtle?

Copy link
Author

Choose a reason for hiding this comment

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

sorry it was hard to read for me that i is greater than j.
I agree with your statement about outer and inner loop, but there is a single loop here and buf[j..i] looked too wierd for me.

np I can remove this, just confirm you want to remove noise.

Copy link
Owner

Choose a reason for hiding this comment

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

Okay, if it's too weird to you then it's probably too weird to others as well. I merged it as is.

It was looking strange to me because there is a single loop, but j was being used in the loop condition instead of i.

@pdeljanov pdeljanov merged commit 31f3ffa into pdeljanov:dev-0.6 Jan 18, 2025
11 checks passed
@sscobici sscobici deleted the core-fix branch January 18, 2025 20:55
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.

2 participants