refactor(AddressRange): create and use a proper type #144
Conversation
| pub fn round_up_to_page(self, page_size: usize) -> Self { | ||
| Self { | ||
| start: self.start, | ||
| end: mm::align_up(self.end, page_size), |
There was a problem hiding this comment.
Could be cool if mm was able to find the page_size by itself
| } | ||
| } | ||
|
|
||
| pub fn iter_pages(self, page_size: usize) -> impl Iterator<Item = usize> { |
There was a problem hiding this comment.
I wonder if we should just provide page_size to new. Are we using AddressRange for something else than pages?
There was a problem hiding this comment.
We don't use it for anything else than pages, I've checked
| pub fn count_pages(&self, page_size: usize) -> usize { | ||
| mm::align_up(self.size(), page_size) / page_size | ||
| } |
hal_riscv64/src/mm/mod.rs
Outdated
| pub fn align_up(addr: usize) -> usize { | ||
| mm::align_up(addr, PageTable::PAGE_SIZE) | ||
| } |
There was a problem hiding this comment.
I think in the future we should just merge hal_core with hal_archXXX with conditional compilation. There is place for some code factoring but that for another time
n1tram1
force-pushed
the
temp/address-range-refactor
branch
from
8488939
to
ff621e5
Compare
|
I agree with all the remarks about |
hal_core/src/lib.rs
Outdated
| } | ||
|
|
||
| impl AddressRange { | ||
| pub fn new(start: usize, end: usize) -> Self { |
There was a problem hiding this comment.
Could be nice to have a constructor from a Range<usize>, the syntax would be a bit more range like: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=38449a783dbfc3192aba75f8c9f6de67
Not sure if that's useful though!
n1tram1
force-pushed
the
temp/address-range-refactor
branch
from
ff621e5
to
4465779
Compare
Previously AddressRange was just a type alias to a usize pair. This caused confusion as sometimes it was (addr, size) or (start, end) and lead to a few confusing debugs. The new type hopefully clear up confusions and make it easy to iterate over the pages in an addressrange.
n1tram1
force-pushed
the
temp/address-range-refactor
branch
from
4465779
to
8348f2d
Compare
Previously AddressRange was just a type alias to a usize pair. This caused confusion as sometimes it was (addr, size) or (start, end) and lead to a few confusing debugs.
The new type hopefully clear up confusions and make it easy to iterate over the pages in an addressrange.