esp-hal
esp-hal copied to clipboard
esp-storage: Fix incorrect usage of MaybeUninit
Submission Checklist 📝
- [x] I have updated existing examples or added new ones (if applicable).
- [x] I have used
cargo xtask fmt-packagescommand to ensure that all changed code is formatted correctly. - [x] My changes were added to the
CHANGELOG.mdin the proper section. - [x] My changes are in accordance to the esp-rs API guidelines
Extra:
- [x] I have read the CONTRIBUTING.md guide and followed its instructions.
Pull Request Details 📖
Description
I found incorrect usage of MaybeUninit while browsing the source that might cause UB. Specifically, calling assume_init_mut on a MaybeUninit that has not been initialized ref.
Testing
WIP
I am not sure how to run the tests, can someone help please
You need to add an entry to the changelog for CI to continue.
You need to add an entry to the changelog for CI to continue.
Do I need to bump the version then?
No, just add an entry for your PR
I just renamed the commit
Wonder what's the status on this? Just checking in :)
Wonder what's the status on this? Just checking in :)
I've been busy, I finish this soon 🙂
Converted to draft for now - set it to Read-for-Review when it's updated
Just checking in here, @DBLouis would you like one of us to take over here to wrap this up? Or do you expect to be able to get back to this in the next couple weeks? No worries either way, would just like to get this across the finish line :)
Just checking in here, @DBLouis would you like one of us to take over here to wrap this up? Or do you expect to be able to get back to this in the next couple weeks? No worries either way, would just like to get this across the finish line :)
Yes it is probably better that someone takes over, I will not have time soon, life got in the way :)
I think the easier solution to get the perf required and to avoid unsafe is to store the flash buffer inside FlashStorage itself? This way it's only initialized once, and this will probably reduce stack usage too.
@bjoernQ are you aware of any reason not to do it like this instead?
I think the easier solution to get the perf required and to avoid unsafe is to store the flash buffer inside
FlashStorageitself? This way it's only initialized once, and this will probably reduce stack usage too.@bjoernQ are you aware of any reason not to do it like this instead?
I think so, yes
Thanks for making us aware of this, I've opened https://github.com/esp-rs/esp-hal/issues/3009 so we don't forget about this.
I'm closing this PR now, please feel free to open a new one if you want to tackle this issue again.