Skip to content

Change to &raw #102

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

Merged
merged 2 commits into from
Feb 3, 2025
Merged

Change to &raw #102

merged 2 commits into from
Feb 3, 2025

Conversation

Riceman2000
Copy link
Contributor

Closes #101

Replaces macro usage and updates docs.

Note this does not touch the crate itself, just docs and examples.

Thanks.

@Riceman2000 Riceman2000 requested a review from a team as a code owner January 28, 2025 02:38
let local_heap: Heap = Heap::empty();
unsafe { local_heap.init(heap_mem.as_ptr() as usize, HEAP_SIZE) }
unsafe { local_heap.init(&raw mut heap_mem as usize, HEAP_SIZE) }
Copy link
Member

Choose a reason for hiding this comment

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

this one doesn't need &raw because it's not a static mut, it's just a plain old array.

Arguably the old code is wrong as well because it used .as_ptr() which gives you a pointer that you're not allowed to use for writing. Could you change this to .as_mut_ptr()?

(and why is heap init taking the address as usize 😭 it should take a pointer. maybe we should change it)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated this, I agree that this should take a pointer.

To give some background on how I came across this issue, I used .as_ptr() and the resulting code ended up being optimized in such a way that it was broken. When using .as_mut_ptr() it would compile and run no problem. I suspect this is to do with provenance and LLVM pulling some tricks because it thinks that memory will never be modified.

Requiring that the pointer be mutable using the type system would remove this kind of issue but it would obviously alter the API.

Docs for .as_ptr() and .as_mut_ptr(). Note the return types of both and that .as_ptr() explicitly returns a const.

Related RFC:
https://rust-lang.github.io/rfcs/3559-rust-has-provenance.html

Thanks for the review!

Copy link
Member

@Dirbaio Dirbaio left a comment

Choose a reason for hiding this comment

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

thanks!

@Dirbaio Dirbaio added this pull request to the merge queue Feb 3, 2025
Merged via the queue into rust-embedded:master with commit 8437c09 Feb 3, 2025
8 checks passed
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.

Use of soft-deprecated macro
2 participants