Re: [PATCH v5 12/20] rust: io: move `Io` methods to extension trait

On Fri Jul 3, 2026 at 7:19 AM BST, Alexandre Courbot wrote:
> On Fri Jun 26, 2026 at 11:45 PM JST, Gary Guo wrote:
>> `Io` trait now has a single required methods with many more provided
>
> nit: s/methods/method
>
>> methods. Provided methods may want to rely on their implementations to not
>> be arbitrarily overridden by implementers for correctness or soundness.
>>
>> Thus, extract these methods to a new trait and provide a blanket
>> implementation. This pattern is used extensively in userspace Rust
>> libraries e.g. `tokio` where `AsyncRead` has minimum methods and
>> `AsyncReadExt` is what users mostly interact with.
>>
>> To avoid changing all user imports, the base trait is renamed to `IoBase`
>> and the newly added trait takes the existing `Io` name.
>>
>> A `size` method is added as an example of methods that users should not
>> override.
>
> WDYM by "an example"? There are already all the I/O accessors that we
> wish not to override; this looks more like a new method is just added.

It wouldn't be incorrect if I/O accessors are overriden, it's just that we don't
anticipate these being overriden. On the other hand, `size` method can be easily
relied upon by other methods (like patch 19 where you mentioned) and it can lead
to UB if it's overriden to something completely wrong. The `size` method is
actually what motivated making this extension trait in the first place.

Best,
Gary


>
> Besides, it is not used until patch 19, so how about introducing it
> there?
>
> <...>
>> @@ -326,6 +323,21 @@ pub trait Io<'a>: Copy {
>>  
>>      /// Return a view that covers the full region.
>>      fn as_view(self) -> <Self::Backend as IoBackend>::View<'a, Self::Target>;
>> +}
>> +
>> +/// Extension trait to provide I/O operation methods to types that implement [`IoBase`].
>> +///
>> +/// This trait provides:
>> +/// - Helper methods for offset validation and address calculation
>> +/// - Fallible (runtime checked) accessors for different data widths
>> +///
>> +/// Which I/O methods are available depends on the associated [`IoBackend`] implementation.
>> +pub trait Io<'a>: IoBase<'a> {
>> +    /// Returns the size of this I/O region.
>> +    #[inline]
>> +    fn size(self) -> usize {
>> +        KnownSize::size(Self::Backend::as_ptr(self.as_view()))
>
> nit: the documentation for `as_ptr` says that the pointer "should be
> used for projection only". Technically we are not projecting here; maybe
> the documentation should be updated to say the pointer must not be
> dereferenced instead.
>
> In any case,
>
> Reviewed-by: Alexandre Courbot <[email protected]>