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 another UB in raw dir #474

Merged
merged 1 commit into from Dec 13, 2022
Merged

Fix another UB in raw dir #474

merged 1 commit into from Dec 13, 2022

Conversation

SUPERCILEX
Copy link
Contributor

@SUPERCILEX SUPERCILEX commented Dec 5, 2022

This kind of sucks, but we can't use for loops until LendingIterator becomes a thing in the stdlib. For now, I elected to simply not implement the Iterator interface and make people manually call next.

Code that compiled before:

let mut buf = Vec::with_capacity(1000);
let mut other = Vec::new();
for x in RawDir::new(cwd(), buf.spare_capacity_mut()) {
    other.push(x.unwrap());
}

Code that no longer compiles:

let mut buf = Vec::with_capacity(1000);
let mut other = Vec::new();
let mut iter = RawDir::new(crate::fs::cwd(), buf.spare_capacity_mut());
while let Some(entry) = iter.next() {
    other.push(entry.unwrap());
}

Closes #472

Signed-off-by: Alex Saveau <saveau.alexandre@gmail.com>
@sunfishcode
Copy link
Member

This looks like it'll work for now. Do you expect the API will be evolving beyond this? rustix is at a point where semver-incompatible version bumps are fairly inconvenient. If RawDir wants to evolve, would it make sense to put it behind a cargo flag?

@SUPERCILEX
Copy link
Contributor Author

Do you expect the API will be evolving beyond this?

Beyond being usable in a for loop when that becomes possible? No. And I'm hopeful that can actually be backwards compatible.

@sunfishcode
Copy link
Member

Makes sense, thanks!

@sunfishcode sunfishcode merged commit 811e5ea into bytecodealliance:main Dec 13, 2022
@SUPERCILEX SUPERCILEX deleted the ub2 branch December 28, 2022 22:57
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.

URGENT: patch out RawDir for the next release too
2 participants