[PATCH] rust: Test page_align usize::MAX boundary edges

[PATCH] rust: Test page_align usize::MAX boundary edges

[bshephar]

From: Brendan Shephard <bshephar@bne-home.net>

Update `page_align` doc tests to test `usize::MAX` boundaries
rather than arbitrary middle of `PAGE_SIZE`.

This patch is a follow-up of:
"[PATCH v8] rust: Return Option from page_align and ensure no usize overflow"

Signed-off-by: Brendan Shephard <bshephar@bne-home.net>
---
 rust/kernel/page.rs | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/rust/kernel/page.rs b/rust/kernel/page.rs
index adecb200c654..d602d4231f56 100644
--- a/rust/kernel/page.rs
+++ b/rust/kernel/page.rs
@@ -45,9 +45,10 @@
 /// assert_eq!(page_align(0x1), Some(PAGE_SIZE));
 /// assert_eq!(page_align(PAGE_SIZE + 1), Some(2 * PAGE_SIZE));
 ///
-/// // Requested address causes overflow (returns `None`).
-/// let overflow_addr = usize::MAX - (PAGE_SIZE / 2);
-/// assert_eq!(page_align(overflow_addr), None);
+/// // Testing boundary conditions at the end of the address space.
+/// assert_eq!(page_align(usize::MAX - PAGE_SIZE + 1), Some(usize::MAX - PAGE_SIZE + 1));
+/// assert_eq!(page_align(usize::MAX - PAGE_SIZE + 2), None);
+/// assert_eq!(page_align(usize::MAX), None);
 /// ```
 #[inline(always)]
 pub const fn page_align(addr: usize) -> Option<usize> {

base-commit: 7acc70476f14661149774ab88d3fe23d83ba4249
-- 
2.52.0

Re: [PATCH] rust: Test page_align usize::MAX boundary edges

[Brendan Shephard]

On Tue, Dec 30, 2025 at 07:48:41PM +1000, bshephar@bne-home.net wrote:
> From: Brendan Shephard <bshephar@bne-home.net>
> 
> Update `page_align` doc tests to test `usize::MAX` boundaries
> rather than arbitrary middle of `PAGE_SIZE`.
> 
> This patch is a follow-up of:
> "[PATCH v8] rust: Return Option from page_align and ensure no usize overflow"
> 
> Signed-off-by: Brendan Shephard <bshephar@bne-home.net>
> ---
>  rust/kernel/page.rs | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/rust/kernel/page.rs b/rust/kernel/page.rs
> index adecb200c654..d602d4231f56 100644
> --- a/rust/kernel/page.rs
> +++ b/rust/kernel/page.rs
> @@ -45,9 +45,10 @@
>  /// assert_eq!(page_align(0x1), Some(PAGE_SIZE));
>  /// assert_eq!(page_align(PAGE_SIZE + 1), Some(2 * PAGE_SIZE));
>  ///
> -/// // Requested address causes overflow (returns `None`).
> -/// let overflow_addr = usize::MAX - (PAGE_SIZE / 2);
> -/// assert_eq!(page_align(overflow_addr), None);
> +/// // Testing boundary conditions at the end of the address space.
> +/// assert_eq!(page_align(usize::MAX - PAGE_SIZE + 1), Some(usize::MAX - PAGE_SIZE + 1));
> +/// assert_eq!(page_align(usize::MAX - PAGE_SIZE + 2), None);
> +/// assert_eq!(page_align(usize::MAX), None);
>  /// ```
>  #[inline(always)]
>  pub const fn page_align(addr: usize) -> Option<usize> {
> 
> base-commit: 7acc70476f14661149774ab88d3fe23d83ba4249
> -- 
> 2.52.0
> 
@miguel, Thanks for the pointers in:
https://lore.kernel.org/all/CANiq72kcMLXz=xyZeKC0=j_e0BzJEY3wGpBTTxfJsc6EZhCnXA@mail.gmail.com/
https://lore.kernel.org/rust-for-linux/CANiq72mLPvB_6Ow3bW5-V4-km=RyA59chQ1g1x9qUt2P-zZweg@mail.gmail.com/

Let me know if this addresses the points you were raising in those.
Maybe you just wanted usize::MAX and then one over? The first test
case might be unnecessary. The last two do a fine job of illustrating the
point I think.

Re: [PATCH] rust: Test page_align usize::MAX boundary edges

[Miguel Ojeda]

On Tue, Dec 30, 2025 at 10:55 AM Brendan Shephard <bshephar@bne-home.net> wrote:
>
> @miguel, Thanks for the pointers in:
> https://lore.kernel.org/all/CANiq72kcMLXz=xyZeKC0=j_e0BzJEY3wGpBTTxfJsc6EZhCnXA@mail.gmail.com/
> https://lore.kernel.org/rust-for-linux/CANiq72mLPvB_6Ow3bW5-V4-km=RyA59chQ1g1x9qUt2P-zZweg@mail.gmail.com/
>
> Let me know if this addresses the points you were raising in those.
> Maybe you just wanted usize::MAX and then one over? The first test
> case might be unnecessary. The last two do a fine job of illustrating the
> point I think.

Thanks! Yeah, either way looks good to me. Personally, I tend to like
tests that check both edges.

By the way, the original comment mentioned "overflow" explicitly,
which was good since it conceptually links to the main docs which also
mention it. It also seemed more aligned with the other two comments.
Perhaps you changed it since now one of them doesn't return `None`? In
that case, I would just remove that bit e.g.

    // Requested address causes overflow.

i.e. I think it is clear that the first case is not the overflow one.
But it is not a big deal either way.

By the way, in commit messages, we normally don't use paragraphs like:

    This patch is a follow-up of: "[PATCH v8] rust: Return Option from
page_align and ensure no usize overflow"

Instead, it is better to provide a lore.kernel.org permalink to the
patch or, if the patch was already applied, refer directly to the
commit with the hash and title. However, in cases like this where it
is not really important for the change itself, I would just move that
line below the `---` line so that it is not part of the commit
message.

I hope that helps!

Cheers,
Miguel

Re: [PATCH] rust: Test page_align usize::MAX boundary edges

[Brendan Shephard]

On Tue, Dec 30, 2025 at 11:14:37AM +0100, Miguel Ojeda wrote:
> On Tue, Dec 30, 2025 at 10:55 AM Brendan Shephard <bshephar@bne-home.net> wrote:
> >
> > @miguel, Thanks for the pointers in:
> > https://lore.kernel.org/all/CANiq72kcMLXz=xyZeKC0=j_e0BzJEY3wGpBTTxfJsc6EZhCnXA@mail.gmail.com/
> > https://lore.kernel.org/rust-for-linux/CANiq72mLPvB_6Ow3bW5-V4-km=RyA59chQ1g1x9qUt2P-zZweg@mail.gmail.com/
> >
> > Let me know if this addresses the points you were raising in those.
> > Maybe you just wanted usize::MAX and then one over? The first test
> > case might be unnecessary. The last two do a fine job of illustrating the
> > point I think.
> 
> Thanks! Yeah, either way looks good to me. Personally, I tend to like
> tests that check both edges.
> 
> By the way, the original comment mentioned "overflow" explicitly,
> which was good since it conceptually links to the main docs which also
> mention it. It also seemed more aligned with the other two comments.
> Perhaps you changed it since now one of them doesn't return `None`? In
> that case, I would just remove that bit e.g.
> 
>     // Requested address causes overflow.
> 
Yeah ok, I agree. I like the explicit mention of overflow, and you are
indeed correct in your assumption for why I changed it. I'll change it
back to the original comment.

> i.e. I think it is clear that the first case is not the overflow one.
> But it is not a big deal either way.
> 
> By the way, in commit messages, we normally don't use paragraphs like:
> 
>     This patch is a follow-up of: "[PATCH v8] rust: Return Option from
> page_align and ensure no usize overflow"
> 
Ack. I'll remove this. I found a similar commit message in the history,
but it did feel like an awkward way to handle it. Noted for future
reference.
> Instead, it is better to provide a lore.kernel.org permalink to the
> patch or, if the patch was already applied, refer directly to the
> commit with the hash and title. However, in cases like this where it
> is not really important for the change itself, I would just move that
> line below the `---` line so that it is not part of the commit
> message.
> 
> I hope that helps!
> 
> Cheers,
> Miguel

Thanks, I'll send a new revision.