Re: [PATCH 1/3] rust: add Aliased type

[Christian Schrefl <chrisi.schrefl@gmail.com>]

On 20.01.25 12:04 AM, Miguel Ojeda wrote:
> Hi Christian,
> 
> Thanks for the series! A few quick comments on the documentation...
> 
> On Sun, Jan 19, 2025 at 11:11 PM Christian Schrefl
> <chrisi.schrefl@gmail.com> wrote:
>>
>> This type is useful for cases where a value might be shared with C code
>> but not interpreted by it.
>> In partiquarly this is added to for data that is shared between a Driver
>> and a MiscDevice implementation.
> 
> Typo: partiquarly

Fixed the typos and missing newlines `--codespell` did not catch them.

> Also, normally you would leave an empty line between paragraphs, if
> that is what you intended.
> 
>> Similar to Opaque but guarantees that the value is initialized and the
>> inner value is dropped when Aliased is dropped.
>>
>> This was origianally proposed for the IRQ abstractions [0], but also
> 
> Typo: origianally
> 
> I usually recommend using `checkpatch.pl` with `--codespell` (it may
> have caught these).
>> +/// Stores a value that may be aliased.
>> +///
>> +/// This is similar to `Opaque<T>` but is guaranteed to contain valid data and will
> 
> Would an intra-doc link be possible? Please use then wherever reasonable.

Fixed

> 
>> +/// Call the Drop implementation of T when dropped.
> 
> "Call" -> "call"?
> 
> Also, please format `Drop` and `T` and add the intra-doc link for the former.
> 
> Could we have an example or two for the type? More generally, you have
> some nice comments in the commit message, so it is best to put some of
> that in the actual documentation, which is what most people will read,
> so that they don't need to search for the commit that introduced it.

I added some more to the documentation and also mentioned it in the 
`Opaque` docs.

> 
>> +    }
>> +    /// Create an `Aliased` pin-initializer from the given pin-initializer.
> 
> Newline between methods/functions/etc.
> 
>> +    pub fn try_pin_init<E>(value: impl PinInit<T, E>) -> impl PinInit<Self, E> {
>> +        // SAFETY:
>> +        // In case of an error in value the error is returned, otherwise the slot is fully initialized,
>> +        // since value is initialized and _pin is a Zero sized type.
>> +        // The pin invariants of value are upheld, since no moving occurs.
> 
> For lists of requirements, we typically use bullets, e.g. `  - `. I
> think this applies to another patch too.

Changed it, also fixed the wrapping.

> 
> Also, please format comments with Markdown as well (no need for
> intra-doc links, of course).
> 
>> +    }
>> +    /// Returns a raw pointer to the opaque data.
Changed this to contained data.

> 
> Missing newline?
> 
> Cheers,
> Miguel

In case you want to take a look the current version of the patch is available at:
https://github.com/onestacked/linux/commits/b4/rust_miscdevice_registrationdata/

Cheers
Christian