Re: [PATCH 2/5] rust: iov: add iov_iter abstractions for ITER_DEST

[Benno Lossin]

On Wed Mar 19, 2025 at 9:01 PM CET, Christian Schrefl wrote:
> On 19.03.25 8:14 PM, Andreas Hindborg wrote:
>> "Christian Schrefl" <chrisi.schrefl@xxxxxxxxx> writes:
>>> On 11.03.25 3:25 PM, Alice Ryhl wrote:
>>>> +/// An IO vector that acts as a destination for data.
>>>> +///
>>>> +/// # Invariants
>>>> +///
>>>> +/// Must hold a valid `struct iov_iter` with `data_source` set to `ITER_DEST`. The buffers
>>>> +/// referenced by the IO vector must be valid for writing for the duration of `'data`.
>>>> +///
>>>> +/// Note that if the IO vector is backed by a userspace pointer, it is always considered valid for
>>>> +/// writing.
>>>> +#[repr(transparent)]
>>>> +pub struct IovIterDest<'data> {
>>>> +    iov: Opaque<bindings::iov_iter>,
>>>> +    /// Represent to the type system that this value contains a pointer to writable data it does
>>>> +    /// not own.
>>>> +    _source: PhantomData<&'data mut [u8]>,
>>>> +}
>>>
>>> It might be a bit nicer to add a (private) struct 'IovIter' that implements the common operations.
>>> Then 'IovIterDest' and 'IovIterSource' could store that struct and forward the implementations to
>>> it.
>>> But I'm not sure if that's really much better.
>> 
>> Yea, I was thinking the same. Maybe we could have an `IovInner` and a
>> local `AsIovInner` trait that would give all the shared methods?
>> 
>> I am sure Alice knows the idiomatic way to achieve code sharing here.
>> 
>
> It would also be possible to use generics here.
>
> Something like (Maybe using types instead of the const-generic):
>
> pub struct IovIter<const DEST: bool> {
>    ...
> }
>
> impl <const DEST: bool> IovIter<DEST> {
>    // Common functions
> }
>
> impl IovIter<false> {
>    // Source functions
> }
>
> impl IovIter<false> {
>    // Dest functions
> }

I would prefer generic types, so `IovIter<Dest>` similar to what Danilo
implemented for `Device<Core>` and `Device<Normal>`. `false` and `true`
aren't descriptive and you also made a typo above :)

---
Cheers,
Benno