Fix bound checking in the NorFlash::erase implementation of FlashRegion#4055
Fix bound checking in the NorFlash::erase implementation of FlashRegion#4055wzhd wants to merge 1 commit intoesp-rs:mainfrom
Conversation
| } | ||
|
|
||
| if !self.range().contains(&address_to) { | ||
| if !address_to <= self.range().end { |
There was a problem hiding this comment.
needs to be !(address_to <= self.range().end) or even better address_to > self.range().end
|
Thanks! Ideally, we would add tests for this but that would require implementing NorFlash for MockFlash the |
|
According to the documentation:
That's a closed-closed interval. Is our current check really incorrect? |
Allow the exclusive address_to to be equal to the exclusive upper bound of a partition.
|
The |
|
Maybe we should then ask for clarification on the documentation, instead of guessing something and risking doing the wrong thing. This may be a case where the rest of our implementation is wrong, but the documentation is right. If we don't know, we shouldn't change behaviour. I guess the helper is a good hint, IMO we still should be sure. |
|
New commits in main has made this PR unmergable. Please resolve the conflicts. |
Submission Checklist 📝
cargo xtask fmt-packagescommand to ensure that all changed code is formatted correctly.CHANGELOG.mdin the proper section.Extra:
Pull Request Details 📖
Description
address_tocould be just past the range, being exactly equal to the end that is exclusive.Testing
I configured littlefs to use
partition size / erase sizeblocks, and got anOutOfBoundserror when it tried to erase the last block.