Re: [PATCH 2/2] rust: page: add method to copy data between safe pages

From: Andreas Hindborg

Date: Sun Feb 15 2026 - 18:47:20 EST


"Miguel Ojeda" <miguel.ojeda.sandonis@xxxxxxxxx> writes:

> On Sun, Feb 15, 2026 at 9:04 PM Andreas Hindborg <a.hindborg@xxxxxxxxxx> wrote:
>>
>> + /// Copies data from this page to another page at the specified offset.
>> + ///
>> + /// # Arguments
>> + ///
>> + /// - `dst` - The destination page to copy data to.
>> + /// - `offset` - The byte offset within both pages where copying starts.
>> + /// - `len` - The number of bytes to copy.
>
> We generally try to avoid this kind of argument-by-argument docs
> unless they are really needed.

Why?

>
> For instance, would this suffice?
>
> /// Copies `len` bytes from this page to another one at the
> specified byte offset.
> ///
> /// Copying starts within both pages at the same offset.
>
>> + /// ```
>> + /// # use kernel::page::SafePage;
>> + /// # use kernel::alloc::flags::GFP_KERNEL;
>> + /// let mut src_page = SafePage::alloc_page(GFP_KERNEL)?;
>> + /// let mut dst_page = SafePage::alloc_page(GFP_KERNEL)?;
>> + /// src_page.copy_to_page(dst_page.get_pin_mut(), 0, 1024)?;
>> + /// # Ok::<(), kernel::error::Error>(())
>> + /// ```
>
> Could we show some error cases?
>
> In addition, why could the test fail here? i.e. if you use `?` in the
> "main line", then it means this could fail for reasons outside the
> test. If it cannot, please assert it instead.
>
> Also, couldn't you assert that some bytes were copied as expected?
> Could you show an error case with e.g. an out of bounds case?

I can demo an out of bounds error, sure.

>
>> + // - By type invariant and existence of shared reference, there are no other writes to
>> + // `src` during this call.
>
> If you use the type invariant here that you promise not to break
> above, isn't it circular logic? Why someone couldn't have another
> `&self` and call this?

Writes require a mutable reference. There cannot be a mutable reference
while we have a shared reference.


Best regards,
Andreas Hindborg