Re: [libgpiod][PATCH 1/3] bindings: rust: fix soundness of line_info modeling

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 28-09-23, 14:27, Erik Schilling wrote:
> On Thu Sep 28, 2023 at 1:27 PM CEST, Viresh Kumar wrote:
> > > -    /// Get the Line info object associated with an event.
> > > -    pub(crate) fn new_from_event(info: *mut gpiod::gpiod_line_info) -> Result<Self> {
> > > -        Info::new_internal(info, true)
> > > +    fn as_raw_ptr(&self) -> *mut gpiod::gpiod_line_info {
> > > +        self as *const _ as *mut _
> >
> > What's wrong with keeping `_info` as `info` in the structure and using it
> > directly instead of this, since this is private anyway ?

Ahh, I missed that it is not *mut anymore. Shouldn't we mark it with & as well,
since it is a reference to the gpiod structure ? Something like ? (I must admit
that I have forgotten a lot of Rust during my long absence from work :)).

    _info: &'a gpiod::gpiod_line_info,


> We would still need to cast it the same way. One _could_ write:
> 
>     fn as_raw_ptr(&self) -> *mut gpiod::gpiod_line_info {
>         &self.info as *const _ as *mut _
>     }

Can we use deref to just do this magically for us in this file somehow ?

> But the cast dance is still required since we need a *mut, but start
> with a readonly reference.
> 
> This is required since libgpiod's C lib keeps the struct internals
> opaque and does not make guarantees about immutable datastructures for
> any API calls.
> 
> Technically, the 1:1 mapping of this to Rust would be to restrict the
> entire API to `&mut self`. One could do that - it would probably allow
> us to advertise the structs as `Sync` - but it would require consumers
> to declare all libgpiod-related variables as `mut`.

-- 
viresh



[Index of Archives]     [Linux SPI]     [Linux Kernel]     [Linux ARM (vger)]     [Linux ARM MSM]     [Linux Omap]     [Linux Arm]     [Linux Tegra]     [Fedora ARM]     [Linux for Samsung SOC]     [eCos]     [Linux Fastboot]     [Gcc Help]     [Git]     [DCCP]     [IETF Announce]     [Security]     [Linux MIPS]     [Yosemite Campsites]

  Powered by Linux