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

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

 



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.

> 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.

> 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().

--
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