Re: [PATCH v6 1/1] rust: introduce abstractions for fwctl

[Danilo Krummrich]

On Mon Jul 6, 2026 at 6:19 AM CEST, Alexandre Courbot wrote:
>> +impl<'a, T: Operations> Registration<'a, T> {
>> +    /// Register a previously allocated fwctl device with the given registration data.
>> +    ///
>> +    /// The `reg_data` is owned by the registration and accessible during callbacks via
>> +    /// `Device::registration_data_unchecked()`.
>> +    ///
>> +    /// # Safety
>> +    ///
>> +    /// Callers must not `mem::forget()` the returned [`Registration`] or otherwise prevent its
>> +    /// [`Drop`] implementation from running, since `fwctl_unregister` must be called before the
>> +    /// parent device is unbound.
>> +    ///
>> +    /// `dev` must be an unregistered [`Device`] that is not associated with any live
>> +    /// [`Registration`], and no other thread may attempt to register the same device concurrently.
>> +    pub unsafe fn new(
>> +        _parent: &'a device::Device<device::Bound>,
>
> Why do we need `_parent`? Is it to provide a proof that the parent
> device is bound by the time of registration? If so, there is an obvious
> loophole: this method will accept any bound device, not necessarily the
> actual parent that was passed to `Device::new`.
>
> We should either store an `ARef<device::Device>` in `fwctl::Device` to
> perform the check at runtime, or add a requirement in the `Safety`
> paragraph that `_parent` must be the actual parent, since the method is
> already `unsafe` anyway.

Neither is needed, the fwctl::Device already carries a parent device pointer
(available through AsRef in Rust).

This function should just compare dev.as_ref() with parent (which I thought was
already the case in a previous version).