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) -> OptionSelf::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.