From 8e85c6ee80aad46acf9053910dc36b0f05d4c407 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mateusz=20Ma=C4=87kowski?= Date: Mon, 19 Oct 2026 14:10:02 +0000 Subject: [PATCH] feat(model)!: introduce DeviceTreeNodeRef This change adds `as_read_only` method that returns a reference struct pointing to the `DeviceTreeNode` and removes `impl Node for &DeviceTreeNode`. There are two main reasons for making this change: * Unifying `FdtNodeMut` and `DeviceTreeNode` - since `FdtNodeMut` contains a mutable reference, it cannot be `Copy`, so for the `Node` trait implementation it exposes its own `as_read_only` method. * This allows us to properly store `parent_address_space` in objects returned by `child()`, `find_node`, etc. without any lifetime issues, self-referential structs, mutability problems, or similar. The main tradeoff is convenience - the users would need to call `as_read_only()` for every reading operation. To avoid this, convenience wrapper methods are added and unified via a macro shared by `FdtNodeMut` and `DeviceTreeNode`. --- src/fdt_mut/node.rs | 42 ++++-------------------- src/model.rs | 14 +++++--- src/model/node.rs | 77 +++++++++++++++++++++++++++++++++++++------- src/model/overlay.rs | 18 +++++------ src/model/writer.rs | 6 ++-- src/overlay.rs | 6 ++-- src/util.rs | 47 +++++++++++++++++++++++++++ 7 files changed, 145 insertions(+), 65 deletions(-) diff --git a/src/fdt_mut/node.rs b/src/fdt_mut/node.rs index f67bf0c..bfd55ac 100644 --- a/src/fdt_mut/node.rs +++ b/src/fdt_mut/node.rs @@ -20,6 +20,7 @@ use crate::fdt_mut::buffer::FdtBuffer; use crate::fdt_mut::property::FdtPropMutIter; use crate::fdt_mut::{FdtMut, FdtPropertyMut}; use crate::standard::{AddressSpaceProperties, NodeStandard}; +use crate::util::impl_read_only_node_methods; use crate::{Node, Property, ToPropertyValue}; /// A mutable device tree node. @@ -280,41 +281,12 @@ impl FdtNodeMut<'_, B> { self.as_read_only().offset() } - /// Returns the name of this node. - #[must_use] - pub fn name(&self) -> &str { - self.as_read_only().name() - } - - /// Returns the name of this node without the unit address, if any. - #[must_use] - pub fn name_without_address(&self) -> &str { - self.as_read_only().name_without_address() - } - - /// Returns the property with the given name, if any. - #[must_use] - pub fn property(&self, name: &str) -> Option> { - self.as_read_only().property(name) - } - - /// Returns an iterator over the properties of this node. - #[must_use] - pub fn properties(&self) -> FdtPropIter<'_> { - self.as_read_only().properties() - } - - /// Returns a child node by its name. - #[must_use] - pub fn child(&self, name: &str) -> Option> { - self.as_read_only().child(name) - } - - /// Returns an iterator over the children of this node. - #[must_use] - pub fn children(&self) -> FdtChildIter<'_> { - self.as_read_only().children() - } + impl_read_only_node_methods!( + node = FdtNode<'_>, + property = FdtProperty<'_>, + properties = FdtPropIter<'_>, + children = FdtChildIter<'_>, + ); } impl Display for FdtNodeMut<'_, B> { diff --git a/src/model.rs b/src/model.rs index 4c32eda..7889b8b 100644 --- a/src/model.rs +++ b/src/model.rs @@ -22,7 +22,7 @@ mod writer; use alloc::vec::Vec; use core::fmt::Display; -pub use node::{DeviceTreeNode, DeviceTreeNodeBuilder}; +pub use node::{DeviceTreeNode, DeviceTreeNodeBuilder, DeviceTreeNodeRef}; pub use property::DeviceTreeProperty; use crate::Node; @@ -92,7 +92,13 @@ impl DeviceTree { } } - /// Finds a node by its path and returns a reference to it. + /// Returns a read-only view of the root node of the device tree. + #[must_use] + pub fn root(&self) -> DeviceTreeNodeRef<'_> { + self.root.as_read_only() + } + + /// Finds a node by its path and returns a read-only view of it. /// /// # Performance /// @@ -112,11 +118,11 @@ impl DeviceTree { /// assert_eq!(child.name(), "child"); /// ``` #[must_use] - pub fn find_node(&self, path: &str) -> Option<&DeviceTreeNode> { + pub fn find_node(&self, path: &str) -> Option> { if !path.starts_with('/') { return None; } - let mut current_node = &self.root; + let mut current_node = self.root(); if path == "/" { return Some(current_node); } diff --git a/src/model/node.rs b/src/model/node.rs index 3851606..3016f77 100644 --- a/src/model/node.rs +++ b/src/model/node.rs @@ -14,6 +14,8 @@ use twox_hash::xxhash64; use super::property::DeviceTreeProperty; use crate::error::ModelError; +use crate::standard::{AddressSpaceProperties, NodeStandard}; +use crate::util::impl_read_only_node_methods; use crate::{Node, Property}; /// A mutable, in-memory representation of a device tree node. @@ -37,13 +39,20 @@ impl Default for DeviceTreeNode { } } -impl<'a> Node<'a> for &'a DeviceTreeNode { +/// A read-only view of a [`DeviceTreeNode`]. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct DeviceTreeNodeRef<'a> { + pub(crate) node: &'a DeviceTreeNode, + pub(crate) parent_address_space: AddressSpaceProperties, +} + +impl<'a> Node<'a> for DeviceTreeNodeRef<'a> { type Property = &'a DeviceTreeProperty; type Properties = private::DeviceTreePropertyRefIter<'a>; type Children = private::DeviceTreeChildIter<'a>; fn name(self) -> &'a str { - &self.name + &self.node.name } /// Finds a property by its name and returns a reference to it. @@ -64,16 +73,16 @@ impl<'a> Node<'a> for &'a DeviceTreeNode { /// assert_eq!(prop.value(), &[1, 2, 3, 4]); /// ``` fn property(self, name: &str) -> Option<&'a DeviceTreeProperty> { - self.properties.get(name) + self.node.properties.get(name) } fn properties(self) -> private::DeviceTreePropertyRefIter<'a> { private::DeviceTreePropertyRefIter { - inner: self.properties.values(), + inner: self.node.properties.values(), } } - /// Finds a child by its name and returns a reference to it. + /// Finds a child by its name and returns a read-only view of it. /// /// # Performance /// @@ -91,9 +100,12 @@ impl<'a> Node<'a> for &'a DeviceTreeNode { /// let child = node.child("child"); /// assert!(child.is_some()); /// ``` - fn child(self, name: &str) -> Option<&'a DeviceTreeNode> { + fn child(self, name: &str) -> Option { if name.contains('@') { - self.children.get(name) + self.node.children.get(name).map(|node| Self { + node, + parent_address_space: self.address_space(), + }) } else { self.children() .find(|child| child.name_without_address() == name) @@ -102,7 +114,8 @@ impl<'a> Node<'a> for &'a DeviceTreeNode { fn children(self) -> private::DeviceTreeChildIter<'a> { private::DeviceTreeChildIter { - inner: self.children.values(), + inner: self.node.children.values(), + parent_address_space: self.address_space(), } } } @@ -110,7 +123,8 @@ impl<'a> Node<'a> for &'a DeviceTreeNode { mod private { use alloc::string::String; - use crate::model::{DeviceTreeNode, DeviceTreeProperty}; + use crate::model::{DeviceTreeNode, DeviceTreeNodeRef, DeviceTreeProperty}; + use crate::standard::AddressSpaceProperties; /// An iterator over the properties of a device tree node (by reference). #[derive(Debug, Clone)] @@ -130,13 +144,18 @@ mod private { #[derive(Debug, Clone)] pub struct DeviceTreeChildIter<'a> { pub inner: indexmap::map::Values<'a, String, DeviceTreeNode>, + pub(crate) parent_address_space: AddressSpaceProperties, } impl<'a> Iterator for DeviceTreeChildIter<'a> { - type Item = &'a DeviceTreeNode; + type Item = DeviceTreeNodeRef<'a>; fn next(&mut self) -> Option { - self.inner.next() + let node = self.inner.next()?; + Some(DeviceTreeNodeRef { + node, + parent_address_space: self.parent_address_space, + }) } } } @@ -353,6 +372,42 @@ impl DeviceTreeNode { pub fn remove_child(&mut self, name: &str) -> Option { self.children.shift_remove(name) } + + /// Returns a read-only view of this node. + /// + /// Note that because [`DeviceTreeNode`] does not store a reference to its + /// parent node, the returned view uses + /// [`AddressSpaceProperties::default()`] as its parent address space. + /// To preserve the parent address space from an ancestor, traverse from + /// [`DeviceTree::root`](crate::model::DeviceTree::root), + /// [`DeviceTree::find_node`](crate::model::DeviceTree::find_node), or a + /// parent node's [`child`](Self::child) / [`children`](Self::children) + /// methods. + /// + /// # Examples + /// + /// ``` + /// use dtoolkit::model::{DeviceTreeNode, DeviceTreeProperty}; + /// use dtoolkit::standard::NodeStandard; + /// + /// let mut node = DeviceTreeNode::new("uart@1000").unwrap(); + /// node.add_property(DeviceTreeProperty::new("compatible", "ns16550a").unwrap()); + /// assert!(node.as_read_only().is_compatible("ns16550a")); + /// ``` + #[must_use] + pub fn as_read_only(&self) -> DeviceTreeNodeRef<'_> { + DeviceTreeNodeRef { + node: self, + parent_address_space: AddressSpaceProperties::default(), + } + } + + impl_read_only_node_methods!( + node = DeviceTreeNodeRef<'_>, + property = &DeviceTreeProperty, + properties = private::DeviceTreePropertyRefIter<'_>, + children = private::DeviceTreeChildIter<'_>, + ); } impl DeviceTreeNode { diff --git a/src/model/overlay.rs b/src/model/overlay.rs index ab30a80..5e2c09d 100644 --- a/src/model/overlay.rs +++ b/src/model/overlay.rs @@ -16,7 +16,7 @@ use alloc::vec::Vec; use crate::error::OverlayError; use crate::fdt::Fdt; -use crate::model::{DeviceTree, DeviceTreeNode, DeviceTreeProperty}; +use crate::model::{DeviceTree, DeviceTreeNode, DeviceTreeNodeRef, DeviceTreeProperty}; use crate::overlay::{ FixupLocation, Fragment, FragmentTarget, NODE_FIXUPS, NODE_LOCAL_FIXUPS, NODE_SYMBOLS, Overlay, PHANDLE_PROPS, get_phandle, @@ -54,7 +54,7 @@ impl<'a> OverlayApplier<'a> { phandles: &mut BTreeMap, max_ph: &mut u32, ) { - if let Some(p) = get_phandle(node) { + if let Some(p) = get_phandle(node.as_read_only()) { *max_ph = (*max_ph).max(p); let p_str = if current_path.is_empty() { "/".to_string() @@ -63,7 +63,7 @@ impl<'a> OverlayApplier<'a> { }; phandles.insert(p, p_str); } - for child in node.children() { + for child in node.children.values() { let old_len = current_path.len(); if !current_path.ends_with('/') { current_path.push('/'); @@ -103,7 +103,7 @@ impl<'a> OverlayApplier<'a> { )?; let overlay_view = Overlay { - node: &overlay.root, + node: overlay.root(), }; let mut fragment_targets = Vec::new(); @@ -182,7 +182,7 @@ impl<'a> OverlayApplier<'a> { fn resolve_fragment_target_path( &self, - fragment: Fragment<&DeviceTreeNode>, + fragment: Fragment>, ) -> Result { match fragment.target()? { FragmentTarget::Phandle(phandle) => { @@ -279,11 +279,11 @@ fn update_phandles_recursive( phandles: &mut BTreeMap, max_ph: &mut u32, ) { - if let Some(p) = get_phandle(node) { + if let Some(p) = get_phandle(node.as_read_only()) { *max_ph = (*max_ph).max(p); phandles.insert(p, current_path.clone()); } - for child in node.children() { + for child in node.children.values() { let old_len = current_path.len(); if !current_path.ends_with('/') { current_path.push('/'); @@ -387,7 +387,7 @@ fn update_local_references( target_prop.set_value(new_value); } - for fixup_child in fixup_node.children() { + for fixup_child in fixup_node.children.values() { let child_name = fixup_child.name(); let target_child = target_node @@ -431,7 +431,7 @@ fn resolve_external_fixups( .find_node_mut(&target_path) .ok_or_else(|| OverlayError::TargetNotFound(target_path.clone()))?; - let phandle = if let Some(p) = get_phandle(&*target_node) { + let phandle = if let Some(p) = get_phandle(target_node.as_read_only()) { p } else { *max_phandle = max_phandle diff --git a/src/model/writer.rs b/src/model/writer.rs index c1022e7..c7b249a 100644 --- a/src/model/writer.rs +++ b/src/model/writer.rs @@ -13,13 +13,13 @@ use alloc::vec::Vec; use zerocopy::IntoBytes; +use crate::Property; use crate::fdt::{ FDT_BEGIN_NODE, FDT_END, FDT_END_NODE, FDT_MAGIC, FDT_TAGSIZE, Fdt, FdtHeader, FdtPropertyHeader, }; use crate::memreserve::MemoryReservation; use crate::model::{DeviceTree, DeviceTreeNode, DeviceTreeProperty}; -use crate::{Node, Property}; // https://devicetree-specification.readthedocs.io/en/latest/chapter5-flattened-format.html#header const LAST_VERSION: u32 = 17; @@ -111,7 +111,7 @@ impl DeviceTree { size += Self::calculate_prop_size(string_map, prop); } - for child in node.children() { + for child in node.children.values() { size += Self::calculate_node_size(string_map, child); } @@ -152,7 +152,7 @@ impl DeviceTree { Self::write_prop(dtb, string_map, prop); } - for child in node.children() { + for child in node.children.values() { Self::write_node(dtb, string_map, child); } diff --git a/src/overlay.rs b/src/overlay.rs index ed87e38..d609b68 100644 --- a/src/overlay.rs +++ b/src/overlay.rs @@ -312,17 +312,17 @@ mod tests { #[cfg(feature = "write")] fn test_get_phandle() { let mut node = DeviceTreeNode::new("test").unwrap(); - assert_eq!(get_phandle(&node), None); + assert_eq!(get_phandle(node.as_read_only()), None); node.add_property( DeviceTreeProperty::new("linux,phandle", 42u32.to_be_bytes().to_vec()).unwrap(), ); - assert_eq!(get_phandle(&node), Some(42)); + assert_eq!(get_phandle(node.as_read_only()), Some(42)); // phandle should take precedence if both are present node.add_property( DeviceTreeProperty::new("phandle", 100u32.to_be_bytes().to_vec()).unwrap(), ); - assert_eq!(get_phandle(&node), Some(100)); + assert_eq!(get_phandle(node.as_read_only()), Some(100)); } } diff --git a/src/util.rs b/src/util.rs index 1a51fee..c38728d 100644 --- a/src/util.rs +++ b/src/util.rs @@ -6,3 +6,50 @@ pub(crate) fn name_without_address(name: &str) -> &str { name } } + +macro_rules! impl_read_only_node_methods { + ( + node = $node:ty, + property = $prop:ty, + properties = $props:ty, + children = $children:ty $(,)? + ) => { + /// Returns the name of this node. + #[must_use] + pub fn name(&self) -> &str { + self.as_read_only().name() + } + + /// Returns the name of this node without the unit address, if any. + #[must_use] + pub fn name_without_address(&self) -> &str { + self.as_read_only().name_without_address() + } + + /// Returns the property with the given name, if any. + #[must_use] + pub fn property(&self, name: &str) -> Option<$prop> { + self.as_read_only().property(name) + } + + /// Returns an iterator over the properties of this node. + #[must_use] + pub fn properties(&self) -> $props { + self.as_read_only().properties() + } + + /// Returns a child node by its name. + #[must_use] + pub fn child(&self, name: &str) -> Option<$node> { + self.as_read_only().child(name) + } + + /// Returns an iterator over the children of this node. + #[must_use] + pub fn children(&self) -> $children { + self.as_read_only().children() + } + }; +} + +pub(crate) use impl_read_only_node_methods;