Re: [PATCH V6 3/8] libgpiod: Add rust wrapper crate

[Kent Gibson <warthog618@xxxxxxxxx>]

On Thu, Sep 29, 2022 at 09:37:40AM +0200, Bartosz Golaszewski wrote:
> On Thu, Sep 29, 2022 at 8:54 AM Viresh Kumar <viresh.kumar@xxxxxxxxxx> wrote:
> >
> > On 28-09-22, 19:54, Bartosz Golaszewski wrote:
> > > On Wed, Sep 28, 2022 at 5:17 PM Viresh Kumar <viresh.kumar@xxxxxxxxxx> wrote:
> > > > Hmm, so what exactly do we want to do here then ?
> > > >
> > > > - Don't allow events to be referenced ? i.e. make event_clone() the default
> > > >   behavior ?
> > > >
> > >
> > > God no, that would be wasteful.
> > >
> > > > - Don't allow read_edge_event() to be called twice for a buffer ? that will be
> > > >   inefficient though.
> > > >
> > >
> > > Not good either.
> >
> > As I expected for both of them :)
> >
> > > > - Somehow guarantee that reference to all the events are dropped before issuing
> > > >   read_edge_event() again, else make it fail ? I am not sure how straight
> > > >   forward that can be though.
> > >
> > > In C++ the preferred way is to do buffer.get_event(0) which will
> > > return a constant reference. If you store that reference as const
> > > edge_event& ev = buffer.get_event(0) and reuse it after rereading into
> > > that buffer and the program crashes - that's on you. In most cases you
> > > should just do buffer.get_event(0).line_offset() etc. If you do:
> > >
> > > edge_event event = buffer.get_event(0);
> > >
> > > You'll copy the event and it will survive the overwriting of the buffer.
> >
> > Right, same happens here.
> >
> > > I'm a Rust beginner but my understanding is that the whole idea of the
> > > language design is to *not* allow a situation where the program can
> > > crash. It should be detected at build-time. We must not rely on
> > > "contracts" defined by documentation.
> >
> > If everything was written in Rust, then this problem won't occur for sure. But
> > in this case part of the code is available via FFI (foreign function interface)
> > and they guarantees are a bit limited there and depend on what the FFI
> > guarantees.
> >
> > > Is there a way to invalidate a reference in Rust? Have a small (cheap)
> > > object in the buffer which the event references and which would get
> > > dropped when reading into the buffer?
> >
> > I am not sure. There are locks, but then they have a cost.
> >
> 
> I'm not talking about locking, this should be left to the user of the module.
> 
> Can we force-drop an object still referenced by other objects in Rust?
> This is what I had in mind - a small, dummy, cheap object inside the
> buffer that's created when reading into the buffer. Each even would
> reference it and then Rust would not allow us to drop it as long as
> there are references to it. Does it make sense? Is that possible?
> 

No, Rust will explicitly prevent you from dropping referenced objects.

But is this the sort of thing you mean:

use std::process::ExitCode;

#[derive(Clone)]
struct Event {
    pub id: u8
}

struct Events {
    b: Vec<Event>
}

impl Events {
    pub fn get(&self, idx: usize) -> Option<&Event> {
        self.b.get(idx)
    }
}

struct Buffer {
    count: u8,
    events: Option<Events>,
}

impl Buffer {
    
    pub fn read(&mut self) -> Result<&Events, ()> {
        self.count += 1;
        self.events = Some(Events{b: vec![Event{id: self.count}]});
        self.events.as_ref().ok_or(())
    }
}

fn main() -> Result<ExitCode, ()>{
    let mut b = Buffer{count: 0, events:None};
    let mut ee = b.read()?;
    let e = ee.get(0);
    println!("{:?}", e.unwrap().id);
    let cloned_e = e.unwrap().clone();
    drop(e); // <-- comment out to try to create a dangling event reference
    ee = b.read()?;
    let e = ee.get(0); // <-- comment out to try to create a dangling event reference
    println!("{:?}", cloned_e.id);
    println!("{:?}", e.unwrap().id);
    Ok(ExitCode::from(42))
}

That is a skeletal proof of concept - the small, dummy, cheap object is
the Vec in Events.  Is that cheap enough? You might be able to replace
that with something cheaper, but Events needs to be able to pull an
Event reference from somewhere (from the borrow checker's PoV) so it
made this demo simpler.

Comment out the two lines to try to carry e across the buffer read().
The compiler will not allow it, as e already borrows from b.

In an actual implementation Event would wrap the C event, and Events.get()
would get the event pointer for the Event and return that as a reference.
The Event clone would call into C, rather than being derived as it is here.

Key points:  Buffer has to own the Events snapshot that is returned by
reference by read().  The return by reference creates a borrow on the
Buffer.  The read() requires a &mut to prevent the Buffer being read
while there are any borrows outstanding.
The Events returns individual events by reference to create a borrow on
the Events to prevent it (and the Buffer) being dropped or modified.
The Event clone returns a concrete event that does not have a borrow of
the Events or Buffer.

There may well be better ways to do this, and you would really want to
do some benchmarking to compare it with the immediate clone option - it
may well be worse, but hopefully it at least demonstrates the semantics
you are after.

Cheers,
Kent.