Re: [PATCH v4] rust: Return Option from page_align and ensure no usize overflow

[Brendan Shephard <bshephar@bne-home.net>]

On Sun, Nov 30, 2025 at 01:01:58PM +0100, Miguel Ojeda wrote:
> Hi Brendan,
> 
> This looks much better now with the examples!
> 
> On Sat, Nov 29, 2025 at 1:54 AM Brendan Shephard <bshephar@bne-home.net> wrote:
> >
> > +/// Returns a page aligned [`usize`] in cases where the value can be aligned. Otherwise, returns `None`
> > +/// if the aligned size will overflow a [`usize`].
> 
> [`None`]
> 
> > +/// # Examples
> 
> Newline before `# Examples` header.
> 
> > +/// Assuming a `PAGE_SIZE` of 4096 (0x1000):
> 
> Can we assume that? i.e. these tests run as KUnit tests and we support
> architectures that allow for other sizes, so we may need to guard
> these with a `cfg` or `if` (which may be hidden), or perhaps better,
> you could compute the values based on `PAGE_SIZE`, e.g. something
> like:
> 
>     assert_eq!(page_align(PAGE_SIZE + 1), Some(2 * PAGE_SIZE));
> 
> It would be nice if you can confirm the tests fail (and then work
> again) running them in a kernel config with a different page size:
> 
>     https://docs.kernel.org/rust/testing.html#the-kunit-tests
> 
Yeah, good plan. I rewrote all of these tests to use the PAGE_SIZE
rather than static values, and I've tested this change on Asahi with 16k
PAGE_SIZE.

> > +/// // The check asserts that None is returned when a value is requested within one PAGE_SIZE of
> > +/// // usize::MAX.
> 
> `None`
> `PAGE_SIZE`
> `usize::MAX`
> 
> > +/// let overflow_addr = usize::MAX - (PAGE_SIZE / 2);
> > +/// assert_eq!(page_align(overflow_addr), None);
> 
> Is there a reason for that particular address? If not, perhaps just
> using `usize::MAX` (the maximum value) would be simpler. Or perhaps
> the edge case (the first value that returns `None`) would be better.
> Or even better, both.
> 
Yeah, I originally just used usize::MAX, but I wanted to demonstrate
that a value within 1 PAGE_SIZE of usize::MAX would overflow and result
in the None condition. The function would accept usize::MAX as a valid
addr value, so I thought doing it this way with a clearly named variable
showing the logic might be more representative of the logic.
> Thanks!
> 
> Cheers,
> Miguel

Thanks again. I'll fix up those docs issues and send a new patch with
the modified tests as well.