On Wed, Nov 26, 2025 at 3:00 PM Alexandre Courbot <[email protected]> wrote: > > 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.
That sounds reasonable enough to me. Alice
