Skip to content

Simplify lifetimes#19

Open
felixwrt wants to merge 2 commits intors-embedded:masterfrom
felixwrt:simplify_lifetimes
Open

Simplify lifetimes#19
felixwrt wants to merge 2 commits intors-embedded:masterfrom
felixwrt:simplify_lifetimes

Conversation

@felixwrt
Copy link
Copy Markdown

Hi,

looking through this crate, I noticed that there were lots of lifetime parameters, some of which aren't useful IMO. In this MR, I'm proposing a refactoring to simplify the lifetimes while also making the crate more flexible:

There are several types in this crate that contain a DevTree, which is a wrapper around &[u8]. These types currently contain a reference to the devtree, for example fdt: &'a DevTree<'dt>. This makes it a double reference (&'a &'dt [u8] basically) and the outer reference doesn't provide significant value. I've therefore replaced occurrences of &'a DevTree<'dt> in several types with DevTree<'dt> (changing the attribute from being a reference to being a value). Here's a diff of one of the types I've changed:

-pub struct DevTreeParseIter<'r, 'dt: 'r> {
+pub struct DevTreeParseIter<'dt> {
     pub offset: usize,
-    pub fdt: &'r DevTree<'dt>,
+    pub fdt: DevTree<'dt>,
 }

DevTree has the memory layout of a slice, which is a fat pointer (pointer + length). Storing it by value instead of by reference makes the type containing it slightly larger, but also removes one level of indirection (which is good for cache locality).

The other (more important) effect of this change is that it removes a lifetime parameter from lots of definitions. It also makes the APIs more flexible:

Currently, one needs to create an instance of DevTree which must outlive all iterators created from it. Something like this currently doesn't compile:

#[test]
fn test() {
    let data = [1,2,3, /* ... */];
    let mut iter = {
        let blob = unsafe { DevTree::new(&data).unwrap() };
        DevTreeNodeIter(DevTreeIter::new(&blob))
        //                                ^^^^^ borrowed value does not live long enough
    };

    while let Some(x) = iter.next().unwrap() {
        dbg!(x.name().unwrap());
    }
}

With the proposed refactoring, this works fine:

#[test]
fn test() {
    let data = [1,2,3, /* ... */];
    let mut iter = {
        let blob = unsafe { DevTree::new(&data).unwrap() };
        DevTreeNodeIter(DevTreeIter::new(blob))
    };

    while let Some(x) = iter.next().unwrap() {
        dbg!(x.name().unwrap());
    }
}

DevTree isn't required to outlive DevTreeIter anymore, which is more flexible. The remaining 'dt lifetime makes sure that the data outlives DevTreeIter. Putting the data into the inner scope in the exmple above like this won't compile:

#[test]
fn test() {
    let mut iter = {
        let data = [1,2,3, /* ... */];
        let blob = unsafe { DevTree::new(&data).unwrap() };
        //                               ^^^^^ borrowed value does not live long enough
        DevTreeNodeIter(DevTreeIter::new(blob))
    };

    while let Some(x) = iter.next().unwrap() {
        dbg!(x.name().unwrap());
    }
}

Storing DevTree by value meant that I had to clone it in a couple of places. That's not an issue though IMO as it simply copies the fat pointer (16 bytes on a 64 bit system).

Let me know if you have questions regarding this MR.

Looking forward to hearing from you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant