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

Remove 'static bound requirement on DMA buffers? #1245

Closed
MabezDev opened this issue Mar 6, 2024 · 2 comments · Fixed by #1837
Closed

Remove 'static bound requirement on DMA buffers? #1245

MabezDev opened this issue Mar 6, 2024 · 2 comments · Fixed by #1837
Assignees
Labels
documentation Improvements or additions to documentation peripheral:dma DMA Peripheral RFC Request for comment
Milestone

Comments

@MabezDev
Copy link
Member

MabezDev commented Mar 6, 2024

To be 100% safe, buffers used with the embedded-dma traits must be static. This is because core::mem::forget is safe in Rust, and if a user were to forget a driver whilst a DMA transaction was occurring, the buffer could go out of scope and UB would occur. Not good.

The issue is, is that 'static bounds are a pain to work with, and I think in practice it's unlikely for users to forget our drivers. I think it might be a good idea to have a trade-off here, to remove the 'static bounds to make the drivers easier to use. We can document each driver, and even have a section in our top level docs mentioning our drivers are not leak (forget) safe and must be dropped properly or manually stopped before forgeting.

@MabezDev MabezDev added RFC Request for comment peripheral:dma DMA Peripheral labels Mar 6, 2024
@github-project-automation github-project-automation bot moved this to Todo in esp-rs Mar 6, 2024
@MabezDev
Copy link
Member Author

When we look into this, we should also fix #954 as removing the 'static bound will make it more likely that the buffer could be in cache (which the DMA can't write to).

@tom-borcin tom-borcin added this to the 0.20.0 milestone Jul 15, 2024
@tom-borcin tom-borcin added the documentation Improvements or additions to documentation label Jul 15, 2024
@bjoernQ
Copy link
Contributor

bjoernQ commented Jul 16, 2024

Maybe we should have our own type for buffers which implements Drop and suggest to deny https://rust-lang.github.io/rust-clippy/v0.0.212/index.html#mem_forget

I haven't tried it - just an idea after sleeping over this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation peripheral:dma DMA Peripheral RFC Request for comment
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants