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

[Viresh Kumar <viresh.kumar@xxxxxxxxxx>]

On 17-12-21, 10:31, Viresh Kumar wrote:
> On 16-12-21, 14:59, Bartosz Golaszewski wrote:
> > AFAICT this object will not survive the parent buffer. Take a look at
> > the current development version of the C++ bindings where I play with
> > copy constructors to ensure that.

I looked at bindings/cxx/line.cpp::make_line_event() now, is this what you were
referring to here ?

Based on what I have understood until now, I think the edge event structure can
be modified like this since it is only required to give us certain fixed values:

 pub struct EdgeEvent {
-    event: *mut bindings::gpiod_edge_event,
+    event_type: GpiodEdgeEvent,
+    timestamp: Duration,
+    line_offset: u32,
+    global_seqno: u64,
+    line_seqno: u64
 }

That way it won't depend on the struct EdgeEventBuffer anymore and can be freed
after the buffer is freed.

> Adding back some of the removed code as well.
> 
> +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).

-- 
viresh