Re: [PATCH V2 2/4] libgpiod: Add rust wrappers

[Bartosz Golaszewski <brgl@xxxxxxxx>]

On Fri, Dec 17, 2021 at 6:01 AM Viresh Kumar <viresh.kumar@xxxxxxxxxx> wrote:
>

[snip]

>
> +impl Drop for GpiodEdgeEvent {
> +    /// Free the edge event object and release all associated resources.
> +    fn drop(&mut self) {
> +        unsafe { bindings::gpiod_edge_event_free(self.event) }
> +    }
> +}
>
> While testing this I found a bug few days back and I wonder why
> gpiod_edge_event_free() even exists. The memory for "events" is allocated with
> the buffer and gpiod_edge_event_free() shouldn't try to free a part of that.
> This looks buggy.
>
> Yes I realize that the edge event shouldn't exist past the buffer itself, I will
> try to fix it in a Rusty way (maybe with Arc or something else).
>

No, it's a different story altogether. In C the buffer allocates
memory for events and when you "get" an event, you only have a pointer
to the memory space in the buffer that you must not free. But you can
"copy" an event with gpiod_edge_event_copy() which returns you a deep
copy of the event that will survive the parent and that must be freed
with gpiod_edge_event_free(). This is done so that by default we try
to limit the number of allocations (as there can be a lot of events)
unless the user decides to manually copy the event.

In C++ I used that mechanism together with the buffer's const
event_get() and event's copy assignment operator. "Getting" an event
returns a const reference to the event (still in buffer's memory) but
copying it triggers a deep copy. The memory management is of course
handled by the destructor.

This is not used in Python as speed is no longer a concern and we'd be
creating new python objects anyway. But in Rust, I think it makes
sense to reuse this mechanism.

Bart