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

[Bartosz Golaszewski <brgl@xxxxxxxx>]

On Mon, Sep 26, 2022 at 5:27 PM Viresh Kumar <viresh.kumar@xxxxxxxxxx> wrote:
>
> Hi Bartosz,
>
> On Mon, 26 Sept 2022 at 18:59, Bartosz Golaszewski <brgl@xxxxxxxx> wrote:
>
> > Thanks for being patient with me. :)
>
> I should thank you for being patient with me :)
>
> > > +    /// Read an event stored in the buffer.
> > > +    pub fn event(&self, index: u64) -> Result<edge::Event> {
> > > +        edge::Event::new(&self.ibuffer, index)
> > > +    }
> >
> > In Event's new() you call gpiod_edge_event_buffer_get_event() which
> > returns a pointer to an event stored inside the buffer. There's also
> > the event_clone() function that calls gpiod_edge_event_copy() but I
> > don't see it called anywhere.
>
> I thought that is required if the user is concerned that the buffer will
> be overwritten by a new event, hence make a copy. Is that
> understanding correct? I didn't use it here for that reason, but it can
> be useful to the user expecting a lot of events.
>

Yes, that's one of the use-cases. Copy an event from a buffer before
reading more into it.

> > Should users pay attention to the
> > lifetime of the buffer storing the event?
>
> No.
>
> > Because IMO if the buffer
> > goes out of scope, the program will crash attempting to access the
> > event.
>
> This is where Rust's memory safety comes in. If you see the design of
> the 'structure Event', it saves a reference to the 'struct BufferInternal'.
> The drop() implementation of BufferInternal calls:
> gpiod_edge_event_buffer_free(), but it won't get called unless all the
> references to it have gone out of scope. So untill the time user is still
> using any of the events, the buffer won't get freed. So we will never
> have invalid reference issue here.

You've got to help me out here, just looking at the code doesn't make
things clear for me.

Am I right to understand that each Event holds a reference to the
BufferInternal object and it will only be destroyed when the last
event is dropped? If we read new events into the buffer, does that
create a new BufferInternal? Is that efficient? Aren't we allocating
memory for each new BufferInternal at every read?

>
> > In C++ the events in the buffer can only be accessed as const
> > edge_event& so it's clear we're only holding a reference to an object
> > existing somewhere else. Internally, the object stored in the buffer
> > doesn't copy the edge event, only holds a C pointer to the event
> > structure in struct gpiod_edge_event_buffer. Only upon calling
> > edge_event& edge_event::operator=(const edge_event& other) will we
> > trigger a copy.
> >
> > This way doing `for (const auto& event: buffer)` allows us to iterate
> > over events without doing any additional allocations.
> >
> > Can we reproduce that behavior in Rust? For instance, the above
> > function could return a borrowed reference and then we could have some
> > interface to trigger the copy? Maybe do an implicit copy like in C++?
> > I don't know if that's possible though.
>
> So here in Rust, you clone() normally to make a copy, but the
> standard clone() declaration can't have an error returned and so I had
> to name it event_clone().
>

Bart