Re: [libgpiod v2][PATCH V8 9/9] bindings: rust: Implement iterator for edge events

[Kent Gibson <warthog618@xxxxxxxxx>]

On Fri, Nov 04, 2022 at 05:01:47PM +0530, Viresh Kumar wrote:
> On 02-11-22, 21:16, Kent Gibson wrote:
> > It is generally frowned upon to patch your own patch within the same
> > series, so drop the Suggested-by and squash this into patch 5.
> > In this instance it is very handy for reviewing though, as the other
> > patches look pretty clean to me...
> 
> I kept it separately for exactly this reason, I was planning to merge it
> eventually with the other commits.
> 
> > > +pub struct Events<'a> {
> > > +    buffer: &'a mut Buffer,
> > > +    events: Vec<&'a Event>,
> > > +    index: usize,
> > > +}
> > > +
> > 
> > Events is pub but not documented.
> > 
> > The events attribute is expensive - requiring a dynamic allocation for
> > each snapshot.  And you create it unsized and push into it, which could
> > require resizing and reallocation.
> > And it is unnecessary - just have Events iterate over the appropriate
> > indicies and get the ref from the buffer.
> 
> I had to do it this way as I wasn't able to get around an error I think. I
> updated the code now based on suggestions, in my v9 branch, and I get it again:
> 
> error: lifetime may not live long enough
>   --> libgpiod/src/event_buffer.rs:51:9
>    |
> 45 | impl<'a> Iterator for Events<'a> {
>    |      -- lifetime `'a` defined here
> ...
> 48 |     fn next(&mut self) -> Option<Self::Item> {
>    |             - let's call the lifetime of this reference `'1`
> ...
> 51 |         event
>    |         ^^^^^ associated function was supposed to return data with lifetime `'a` but it is returning data with lifetime `'1`
> 
> error: could not compile `libgpiod` due to previous error
> 
> I will try to fix this on Monday now, unless you have an answer for me by then
> :)


     /// Read an event stored in the buffer.
-    fn event(&mut self, index: usize) -> Result<&Event> {
+    fn event<'a>(&mut self, index: usize) -> Result<&'a Event> {
         if self.events[index].is_null() {
             // SAFETY: The `gpiod_edge_event` returned by libgpiod is guaranteed to live as long
             // as the `struct Event`.

There are still other things that need to be fixed after that, but that
fixes that particular problem.

Cheers,
Kent.