Fixes from code review.

Author: qwandor

src/error.rs

@@ -37,11 +37,9 @@ pub enum FdtError {
     },
     /// Tried to convert part of a prop-encoded-array property to a type which
     /// was too small.
-    #[error("{field} too big for chosen type ({cells} cells)")]
+    #[error("prop-encoded-array field too big for chosen type ({cells} cells)")]
     TooManyCells {
-        /// The name of the field.
-        field: &'static str,
-        /// The value of the relevant `#address-cells` property.
+        /// The number of (32-bit) cells in the field.
         cells: usize,
     },
 }

src/fdt/node.rs

@@ -13,26 +13,24 @@ use core::fmt;
 use super::{FDT_TAGSIZE, Fdt, FdtToken};
 use crate::error::FdtParseError;
 use crate::fdt::property::{FdtPropIter, FdtProperty};
-use crate::standard::{DEFAULT_ADDRESS_CELLS, DEFAULT_SIZE_CELLS};
+use crate::standard::AddressSpaceProperties;
 
 /// A node in a flattened device tree.
 #[derive(Debug, Clone, Copy)]
 pub struct FdtNode<'a> {
     pub(crate) fdt: Fdt<'a>,
     pub(crate) offset: usize,
-    /// The `#size-cells` property of this node's parent node.
-    pub(crate) parent_size_cells: u32,
-    /// The `#address-cells` property of this node's parent node.
-    pub(crate) parent_address_cells: u32,
+    /// The `#address-cells` and `#size-cells` properties of this node's parent
+    /// node.
+    pub(crate) parent_address_space: AddressSpaceProperties,
 }
 
 impl<'a> FdtNode<'a> {
     pub(crate) fn new(fdt: Fdt<'a>, offset: usize) -> Self {
         Self {
             fdt,
             offset,
-            parent_address_cells: DEFAULT_ADDRESS_CELLS,
-            parent_size_cells: DEFAULT_SIZE_CELLS,
+            parent_address_space: AddressSpaceProperties::default(),
         }
     }
 
@@ -259,8 +257,7 @@ enum FdtChildIter<'a> {
     Running {
         fdt: Fdt<'a>,
         offset: usize,
-        address_cells: u32,
-        size_cells: u32,
+        address_space: AddressSpaceProperties,
     },
     Error,
 }
@@ -271,11 +268,7 @@ impl<'a> Iterator for FdtChildIter<'a> {
     fn next(&mut self) -> Option<Self::Item> {
         match self {
             Self::Start { node } => {
-                let address_cells = match node.address_cells() {
-                    Ok(value) => value,
-                    Err(e) => return Some(Err(e)),
-                };
-                let size_cells = match node.size_cells() {
+                let address_space = match node.address_space() {
                     Ok(value) => value,
                     Err(e) => return Some(Err(e)),
                 };
@@ -292,17 +285,15 @@ impl<'a> Iterator for FdtChildIter<'a> {
                 *self = Self::Running {
                     fdt: node.fdt,
                     offset,
-                    address_cells,
-                    size_cells,
+                    address_space,
                 };
                 self.next()
             }
             Self::Running {
                 fdt,
                 offset,
-                address_cells,
-                size_cells,
-            } => match Self::try_next(*fdt, offset, *address_cells, *size_cells) {
+                address_space,
+            } => match Self::try_next(*fdt, offset, *address_space) {
                 Some(Ok(val)) => Some(Ok(val)),
                 Some(Err(e)) => {
                     *self = Self::Error;
@@ -319,8 +310,7 @@ impl<'a> FdtChildIter<'a> {
     fn try_next(
         fdt: Fdt<'a>,
         offset: &mut usize,
-        parent_address_cells: u32,
-        parent_size_cells: u32,
+        parent_address_space: AddressSpaceProperties,
     ) -> Option<Result<FdtNode<'a>, FdtParseError>> {
         loop {
             let token = match fdt.read_token(*offset) {
@@ -337,8 +327,7 @@ impl<'a> FdtChildIter<'a> {
                     return Some(Ok(FdtNode {
                         fdt,
                         offset: node_offset,
-                        parent_address_cells,
-                        parent_size_cells,
+                        parent_address_space,
                     }));
                 }
                 FdtToken::Prop => {

src/fdt/property.rs

@@ -319,8 +319,8 @@ impl<'a> Iterator for FdtStringListIterator<'a> {
 /// An integer value split into several big-endian u32 parts.
 ///
 /// This is generally used in prop-encoded-array properties.
-#[derive(Clone, Copy, Debug, Default, Eq, PartialEq)]
-pub struct Cells<'a>(pub &'a [big_endian::U32]);
+#[derive(Clone, Copy, Debug, Default, Eq, Hash, Ord, PartialEq, PartialOrd)]
+pub struct Cells<'a>(pub(crate) &'a [big_endian::U32]);
 
 impl Cells<'_> {
     /// Converts the value to the given integer type.
@@ -329,13 +329,11 @@ impl Cells<'_> {
     ///
     /// Returns `FdtError::TooManyCells` if the value has too many cells to fit
     /// in the given type.
-    pub fn to_intsize<T: Default + From<u32> + Shl<usize, Output = T> + BitOr<Output = T>>(
+    pub fn to_int<T: Default + From<u32> + Shl<usize, Output = T> + BitOr<Output = T>>(
         self,
-        field: &'static str,
     ) -> Result<T, FdtError> {
         if size_of::<T>() < self.0.len() * size_of::<u32>() {
             Err(FdtError::TooManyCells {
-                field,
                 cells: self.0.len(),
             })
         } else if let [size] = self.0 {

src/standard.rs

@@ -139,6 +139,20 @@ impl<'a> FdtNode<'a> {
         })
     }
 
+    /// Returns the values of the standard `#address-cells` and `#size_cells`
+    /// properties.
+    ///
+    /// # Errors
+    ///
+    /// Returns an error if a property's name or value cannot be read, or the
+    /// values aren't valid u32s.
+    pub fn address_space(&self) -> Result<AddressSpaceProperties, FdtParseError> {
+        Ok(AddressSpaceProperties {
+            address_cells: self.address_cells()?,
+            size_cells: self.size_cells()?,
+        })
+    }
+
     /// Returns the value of the standard `reg` property.
     ///
     /// # Errors
@@ -147,8 +161,8 @@ impl<'a> FdtNode<'a> {
     /// size of the value isn't a multiple of the expected number of address and
     /// size cells.
     pub fn reg(&self) -> Result<Option<impl Iterator<Item = Reg<'a>> + use<'a>>, FdtError> {
-        let address_cells = self.parent_address_cells as usize;
-        let size_cells = self.parent_size_cells as usize;
+        let address_cells = self.parent_address_space.address_cells as usize;
+        let size_cells = self.parent_address_space.size_cells as usize;
         Ok(if let Some(property) = self.property("reg")? {
             Some(
                 property
@@ -186,7 +200,7 @@ impl<'a> FdtNode<'a> {
                 property
                     .as_prop_encoded_array([
                         self.address_cells()? as usize,
-                        self.parent_address_cells as usize,
+                        self.parent_address_space.address_cells as usize,
                         self.size_cells()? as usize,
                     ])?
                     .map(Range::from_cells),
@@ -210,7 +224,7 @@ impl<'a> FdtNode<'a> {
                 property
                     .as_prop_encoded_array([
                         self.address_cells()? as usize,
-                        self.parent_address_cells as usize,
+                        self.parent_address_space.address_cells as usize,
                         self.size_cells()? as usize,
                     ])?
                     .map(Range::from_cells),
@@ -229,3 +243,21 @@ impl<'a> FdtNode<'a> {
         Ok(self.property("dma-coherent")?.is_some())
     }
 }
+
+/// The `#address-cells` and `#size-cells` properties of a node.
+#[derive(Debug, Clone, Copy)]
+pub struct AddressSpaceProperties {
+    /// The `#address-cells` property.
+    pub address_cells: u32,
+    /// The `#size-cells` property.
+    pub size_cells: u32,
+}
+
+impl Default for AddressSpaceProperties {
+    fn default() -> Self {
+        Self {
+            address_cells: DEFAULT_ADDRESS_CELLS,
+            size_cells: DEFAULT_SIZE_CELLS,
+        }
+    }
+}

src/standard/memory.rs

@@ -88,15 +88,20 @@ pub struct InitialMappedArea {
 }
 
 impl InitialMappedArea {
+    /// Creates an `InitialMappedArea` from an array of three `Cells` containing
+    /// the effective address, physical address and size respectively.
+    ///
+    /// These `Cells` must contain 2, 2 and 1 cells respectively, or the method
+    /// will panic.
     #[expect(
         clippy::unwrap_used,
         reason = "The Cells passed are always the correct size"
     )]
     fn from_cells([ea, pa, size]: [Cells; 3]) -> Self {
         Self {
-            effective_address: ea.to_intsize("effective address").unwrap(),
-            physical_address: pa.to_intsize("physical address").unwrap(),
-            size: size.to_intsize("size").unwrap(),
+            effective_address: ea.to_int().unwrap(),
+            physical_address: pa.to_int().unwrap(),
+            size: size.to_int().unwrap(),
         }
     }
 }

src/standard/ranges.rs

@@ -46,7 +46,7 @@ impl<'a> Range<'a> {
     >(
         &self,
     ) -> Result<T, FdtError> {
-        self.child_bus_address.to_intsize("child-bus-address")
+        self.child_bus_address.to_int()
     }
 
     /// Attempts to return the parent-bus-address as the given type, if it will
@@ -61,7 +61,7 @@ impl<'a> Range<'a> {
     >(
         &self,
     ) -> Result<T, FdtError> {
-        self.parent_bus_address.to_intsize("parent-bus-address")
+        self.parent_bus_address.to_int()
     }
 
     /// Attempts to return the length as the given type, if it will fit.
@@ -72,7 +72,7 @@ impl<'a> Range<'a> {
     pub fn length<T: Default + From<u32> + Shl<usize, Output = T> + BitOr<Output = T>>(
         &self,
     ) -> Result<T, FdtError> {
-        self.length.to_intsize("length")
+        self.length.to_int()
     }
 }
 

src/standard/reg.rs

@@ -34,7 +34,7 @@ impl<'a> Reg<'a> {
     pub fn address<T: Default + From<u32> + Shl<usize, Output = T> + BitOr<Output = T>>(
         self,
     ) -> Result<T, FdtError> {
-        self.address.to_intsize("address")
+        self.address.to_int()
     }
 
     /// Attempts to return the size as the given type, if it will fit.
@@ -45,7 +45,7 @@ impl<'a> Reg<'a> {
     pub fn size<T: Default + From<u32> + Shl<usize, Output = T> + BitOr<Output = T>>(
         self,
     ) -> Result<T, FdtError> {
-        self.size.to_intsize("size")
+        self.size.to_int()
     }
 }
 
@@ -80,10 +80,7 @@ mod tests {
         };
         assert_eq!(
             reg.address::<u32>(),
-            Err(FdtError::TooManyCells {
-                field: "address",
-                cells: 2
-            })
+            Err(FdtError::TooManyCells { cells: 2 })
         );
         assert_eq!(reg.address::<u64>(), Ok(0x1234_5678_abcd_0000));
         assert_eq!(reg.size::<u32>(), Ok(0x1122_3344));

tests/fdt.rs

@@ -6,10 +6,10 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-use dtoolkit::fdt::{Cells, Fdt};
+use dtoolkit::fdt::Fdt;
 #[cfg(feature = "write")]
 use dtoolkit::model::DeviceTree;
-use dtoolkit::standard::{InitialMappedArea, Reg, Status};
+use dtoolkit::standard::{InitialMappedArea, Status};
 
 #[test]
 fn read_child_nodes() {
@@ -135,19 +135,7 @@ fn standard_properties() {
         .unwrap()
         .unwrap()
         .collect::<Vec<_>>();
-    assert_eq!(
-        reg,
-        vec![
-            Reg {
-                address: Cells(&[0x1234_5678.into(), 0x3000.into()]),
-                size: Cells(&[0.into(), 32.into()]),
-            },
-            Reg {
-                address: Cells(&[0.into(), 0xfe00.into()]),
-                size: Cells(&[0.into(), 256.into()]),
-            },
-        ]
-    );
+    assert_eq!(reg.len(), 2);
     assert_eq!(reg[0].address::<u64>().unwrap(), 0x1234_5678_0000_3000);
     assert_eq!(reg[0].size::<u64>().unwrap(), 32);
     assert_eq!(reg[1].address::<u64>().unwrap(), 0xfe00);
@@ -258,13 +246,6 @@ fn memory() {
     assert_eq!(reg.len(), 1);
     assert_eq!(reg[0].address::<u32>().unwrap(), 0x8000_0000);
     assert_eq!(reg[0].size::<u32>().unwrap(), 0x2000_0000);
-    assert_eq!(
-        reg,
-        vec![Reg {
-            address: Cells(&[0x8000_0000.into()]),
-            size: Cells(&[0x2000_0000.into()]),
-        }]
-    );
     assert!(memory.hotpluggable().unwrap());
     assert_eq!(
         memory