On Wed Nov 26, 2025 at 10:36 PM JST, Alice Ryhl wrote: > On Wed, Nov 26, 2025 at 10:22:14PM +0900, Alexandre Courbot wrote: >> On Wed Nov 26, 2025 at 6:54 PM JST, Alice Ryhl wrote: >> > On Wed, Nov 26, 2025 at 09:31:46AM +0900, Alexandre Courbot wrote: >> >> On Tue Nov 25, 2025 at 11:59 PM JST, Alice Ryhl wrote: >> >> > On Tue, Nov 25, 2025 at 3:55 PM Alexandre Courbot <[email protected]> >> >> > wrote: >> >> >> >> >> >> On Tue Nov 25, 2025 at 11:41 PM JST, Alice Ryhl wrote: >> >> >> > On Tue, Nov 25, 2025 at 3:29 PM Alexandre Courbot >> >> >> > <[email protected]> wrote: >> >> >> >> >> >> >> >> On Fri Nov 21, 2025 at 1:04 PM JST, bshephar wrote: >> >> >> >> > return Err(EINVAL); >> >> >> >> > } >> >> >> >> > >> >> >> >> > + let aligned_size = page_align(size); >> >> >> >> >> >> >> >> `page_align` won't panic on overflow, but it will still return an >> >> >> >> invalid size. This is a job for `kernel::ptr::Alignment`, which >> >> >> >> let's >> >> >> >> you return an error when an overflow occurs. >> >> >> > >> >> >> > The Rust implementation of page_align() is implemented as (addr + >> >> >> > (PAGE_SIZE - 1)) & PAGE_MASK, which definitely will panic on overflow >> >> >> > if the appropriate config options are enabled. >> >> >> >> >> >> That's right, I skimmed its code too fast. ^_^; All the more reason to >> >> >> use `Alignment`. >> >> > >> >> > Alignment stores values that are powers of two, not multiples of >> >> > PAGE_SIZE. >> >> >> >> Isn't PAGE_SIZE always a power of two though? >> > >> > Yes it is. Maybe you can elaborate on how you wanted to use Alignment? >> > It sounds like you have something different in mind than what I thought. >> >> I thought we could just do something like this: >> >> use kernel::ptr::{Alignable, Alignment}; >> >> let aligned_size = size >> .align_up(Alignment::new::<PAGE_SIZE>()) >> .ok_or(EOVERFLOW)?; >> >> (maybe we could also have that `Alignment<PAGE_SIZE>` stored as a const >> in `page.rs` for convenience, as it might be used often) > > If you're trying to round up a number to the next multiple of PAGE_SIZE, > then you should use page_align() because that's exactly what the > function does. > > If you think there is something wrong with the design of page_align(), > change it instead of reimplemtning it.
In that case I would suggest that `page_align` returns an `Option` instead of potentially panicking. Does that sound reasonable? I cannot find any user of it in the Rust code for now.
