Re: [PATCH v5 1/2] rust: regulator: add a bare minimum regulator abstraction

[Daniel Almeida]

Hi,

> +    /// Attempts to convert the regulator to an enabled state.
> +    pub fn try_into_enabled(mut self) -> Result<Regulator<Enabled>, Error<Disabled>> {
> +        self.enable_internal()
> +            .map(|()| Regulator {
> +                inner: self.inner,
> +                _phantom: PhantomData,
> +            })
> +            .map_err(|error| Error {
> +                error,
> +                regulator: self,
> +            })
> +    }
> +}
> +
> +impl Regulator<Enabled> {
> +    /// Obtains a [`Regulator`] instance from the system and enables it.
> +    ///
> +    /// This is equivalent to calling `regulator_get_enable()` in the C API.
> +    pub fn get(dev: &Device, name: &CStr) -> Result<Self> {
> +        Regulator::<Disabled>::get_internal(dev, name)?
> +            .try_into_enabled()
> +            .map_err(|error| error.error)
> +    }

I just realized that this is a bug.

The pre-typestate code was using ManuallyDrop<T> here, and it was forgotten on
the newer versions. This means that the destructor for self runs here, which
decreases the refcount by calling regulator_put().

My proposed solution is:

    /// Attempts to convert the regulator to an enabled state.
    pub fn try_into_enabled(mut self) -> Result<Regulator<Enabled>, Error<Disabled>> {
        // We will be transferring the ownership of our regulator_get() count to Regulator<Enabled>
        let mut regulator = ManuallyDrop::new(self);

        regulator.enable_internal()
            .map(|()| Regulator {
                inner: regulator.inner,
                _phantom: PhantomData,
            })
            .map_err(|error| Error {
                error,
                regulator: ManuallyDrop::into_inner(regulator),
            })
    }
}



Alex, Benno, thoughts?

— Daniel