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

Rope code appears to do a string overrun #2236

Closed
graydon opened this issue Apr 19, 2012 · 4 comments · Fixed by #7629
Closed

Rope code appears to do a string overrun #2236

graydon opened this issue Apr 19, 2012 · 4 comments · Fixed by #7629

Comments

@graydon
Copy link
Contributor

graydon commented Apr 19, 2012

In commit 956bc77 I had to disable one of the rope tests. The purpose of that commit was to fix up a bug in the way operator [] works on strings. In particular it used to let you index up to and including the trailing null; now it considers access to the null byte an index overrun and fails.

I tried to figure out exactly what was going on in the rope code for quite a while, and tried adjusting a number of obvious-looking things, but to no avail. I'm not really good with boundary-case arithmetic (are any of us?) and the rope module is a maze of such arithmetic.

Sharper minds requested to find the bug. Meanwhile I marked the rope test in question (char_at1) as ignored. The others seem to work.

@catamorphism
Copy link
Contributor

Paging @Yoric

@Yoric
Copy link
Contributor

Yoric commented Apr 22, 2012

I'll take a look.

@msullivan
Copy link
Contributor

There is some discussion on IRC that maybe we should just pitch the rope code as it exists now.

@Aatch
Copy link
Contributor

Aatch commented Jul 3, 2013

@msullivan agreed. The code is pretty outdated and the work required to modernize it would probably be similar to the work needed to just re-write it.

When I was looking through it, it seemed to be doing some very strange things that I don't think are even close to being safe, so there's that too.

bors added a commit that referenced this issue Jul 7, 2013

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
It's broken/unmaintained and needs to be rewritten to avoid managed
pointers and needless copies. A full rewrite is necessary and the API
will need to be redone so it's not worth keeping this around (#7628).

Closes #2236, #2744
bors added a commit to rust-lang-ci/rust that referenced this issue Sep 22, 2022
rustup

I feel like tests became a *lot* slower. I am not sure what is going on and don't have time to debug right now.
BoxyUwU pushed a commit to BoxyUwU/rust that referenced this issue Feb 11, 2025
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 a pull request may close this issue.

5 participants