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 str::eq_slice off-by-one error #3365

Closed
wants to merge 2 commits into from
Closed

fix str::eq_slice off-by-one error #3365

wants to merge 2 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Sep 3, 2012

The new memcmp-based implementation of str::eq_slice (54a8d69) uses str::as_buf, which, according to its documentation, passes one more than the real length of the string in order to facilitate null-terminator checks. eq_slice doesn't take this into account, and compares the next byte beyond the ends of the slices it is given. For full strings, this is the null terminator, but for slices in the middle of a string, this is whatever the next UTF-8 byte is.

Therefore, code like assert str::view("foobar", 0, 3) == "foo"; fails.

This fix simply subtracts one from the length yielded by as_buf in order to compensate for this.

@brson
Copy link
Contributor

brson commented Sep 4, 2012

Thanks. I think we should merge this fix, and then reconsider the interface to as_buf. This is some confusing behavior and it looks like the only users of that len value don't actually care about the potentially extra null.

@brson
Copy link
Contributor

brson commented Sep 4, 2012

Pushed to incoming.

@brson brson closed this Sep 4, 2012
RalfJung pushed a commit to RalfJung/rust that referenced this pull request Mar 9, 2024
RalfJung pushed a commit to RalfJung/rust that referenced this pull request Mar 9, 2024
jaisnan pushed a commit to jaisnan/rust-dev that referenced this pull request Jul 29, 2024
Dependency upgrade resulting from `cargo update`.
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