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

A few suggestions regarding unsafe code #1170

Closed
joshlf opened this issue Aug 30, 2021 · 6 comments
Closed

A few suggestions regarding unsafe code #1170

joshlf opened this issue Aug 30, 2021 · 6 comments

Comments

@joshlf
Copy link
Contributor

joshlf commented Aug 30, 2021

While reviewing a commit to vendor new versions of various rand crates into Fuchsia's source tree, a reviewer came across a few issues. I'm not sure that they all deserve to be acted on, so I figured I'd just bring them to the authors' attention here and let y'all decide if they're worth opening issues for.

Visit https://fxrev.dev/569847 and click on the "Comments" tab. There are three unresolved comments, each of which contains suggestions about how to improve or remove unsafe code.

cc @Ralith

@josephlr
Copy link
Member

@joshlf it seems like all the comments are now resolved. Other than #1169, which specific comments contain issues that we should fix here.

Also, thank you for looking into this.

@vks
Copy link
Collaborator

vks commented Sep 15, 2021

I think all issues mentioned here are fixed now, except that we still use copy_nonoverlapping for performance reasons (see here).

@dhardy
Copy link
Member

dhardy commented Sep 15, 2021

Yes, but I just noticed a perf regression in #1181 so it may need revisiting.

I also created #1182.

@sneurlax
Copy link

sneurlax commented May 9, 2023

Are all of these issues resolved?

@dhardy
Copy link
Member

dhardy commented May 10, 2023

All code mentioned has been re-written in master so we might as well close this.

Not all of these changes have made it into releases yet, but I'm not aware of any pressing concerns here.

@dhardy dhardy closed this as completed May 10, 2023
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

No branches or pull requests

5 participants