tool(cleanup): add sdf_parse_attribute() functions to decrease duplication

tool/microkit/src/sdf/channels.rs

@@ -9,11 +9,12 @@ use std::rc::Rc;
 
 use super::consts::*;
 use super::pd_vm::ProtectionDomain;
-use super::util::{check_attributes, checked_lookup, loc_string, value_error};
+use super::util::{
+    check_attributes, checked_lookup, loc_string, sdf_parse_attribute,
+    sdf_parse_required_attribute, value_error,
+};
 use super::{SdfNode, SystemDescriptionFile};
 
-use crate::util::str_to_bool;
-
 #[derive(Debug, Clone)]
 pub struct ChannelEnd {
     pub pd: Rc<str>,
@@ -47,7 +48,7 @@ impl ChannelEnd {
 
         check_attributes(xml_sdf, node, &["pd", "id", "pp", "notify", "setvar_id"])?;
         let end_pd = checked_lookup(xml_sdf, node, "pd")?;
-        let end_id = checked_lookup(xml_sdf, node, "id")?.parse::<i64>().unwrap();
+        let end_id: i64 = sdf_parse_required_attribute(xml_sdf, node, "id")?;
 
         if end_id > PD_MAX_ID as i64 {
             return Err(value_error(
@@ -61,25 +62,8 @@ impl ChannelEnd {
             return Err(value_error(xml_sdf, node, "id must be >= 0".to_string()));
         }
 
-        let notify = node
-            .attribute("notify")
-            .map(str_to_bool)
-            .unwrap_or(Some(true))
-            .ok_or_else(|| {
-                value_error(
-                    xml_sdf,
-                    node,
-                    "notify must be 'true' or 'false'".to_string(),
-                )
-            })?;
-
-        let pp = node
-            .attribute("pp")
-            .map(str_to_bool)
-            .unwrap_or(Some(false))
-            .ok_or_else(|| {
-                value_error(xml_sdf, node, "pp must be 'true' or 'false'".to_string())
-            })?;
+        let notify = sdf_parse_attribute(xml_sdf, node, "notify")?.unwrap_or(true);
+        let pp = sdf_parse_attribute(xml_sdf, node, "pp")?.unwrap_or(false);
 
         if let Some(pd) = pds.get(end_pd) {
             let setvar_id = node.attribute("setvar_id").map(ToOwned::to_owned);

tool/microkit/src/sdf/cspace.rs

@@ -7,7 +7,9 @@
 use std::rc::Rc;
 
 use super::consts::*;
-use super::util::{check_attributes, checked_lookup, loc_string, sdf_parse_number, value_error};
+use super::util::{
+    check_attributes, checked_lookup, loc_string, sdf_parse_required_attribute, value_error,
+};
 use super::{SdfLocation, SdfNode, SystemDescriptionFile};
 
 #[derive(Debug, PartialEq, Eq, Copy, Clone)]
@@ -47,7 +49,7 @@ impl CapMap {
 
         let pd = Rc::from(checked_lookup(xml_sdf, node, "pd")?);
 
-        let slot = sdf_parse_number(checked_lookup(xml_sdf, node, "slot")?, node)?;
+        let slot: u64 = sdf_parse_required_attribute(xml_sdf, node, "slot")?;
 
         if slot == 0 {
             return Err(value_error(

tool/microkit/src/sdf/domains.rs

@@ -9,7 +9,9 @@ use std::num::NonZero;
 
 use sel4_capdl_initializer_types::{DomainSchedDuration, DomainSchedEntry};
 
-use super::util::{check_attributes, checked_lookup, loc_string, sdf_parse_number, value_error};
+use super::util::{
+    check_attributes, checked_lookup, loc_string, parse_number, sdf_parse_attribute, value_error,
+};
 use super::{SdfNode, SystemDescriptionFile};
 
 use crate::Config;
@@ -144,12 +146,9 @@ impl Domains {
 
         let name = checked_lookup(xml_sdf, node, "name")?.to_string();
 
-        let domain_id = node
-            .attribute("id")
-            .map(|s| sdf_parse_number(s, node))
-            .transpose()?
-            .map(|n| {
-                if n >= config.num_domains.into() {
+        let domain_id = sdf_parse_attribute(xml_sdf, node, "id")?
+            .map(|n: u8| {
+                if n >= config.num_domains {
                     Err(value_error(
                         xml_sdf,
                         node,
@@ -160,8 +159,7 @@ impl Domains {
                         ),
                     ))
                 } else {
-                    Ok(n.try_into()
-                        .expect("num_domains is u8 so by if above this is OK"))
+                    Ok(n)
                 }
             })
             .transpose()?;
@@ -177,18 +175,12 @@ impl Domains {
     ) -> Result<Domains, String> {
         check_attributes(xml_sdf, node, &["index_shift", "start_index"])?;
 
-        let schedule_start_index = node
-            .attribute("start_index")
-            .map(|s| sdf_parse_number(s, node))
-            .transpose()?
+        let schedule_start_index: u64 = sdf_parse_attribute(xml_sdf, node, "start_index")?
             // The domain schedule is only started when the start index is Some(...)
             // so even when not specified we default to a start index of zero.
             .unwrap_or(0);
 
-        let schedule_index_shift = node
-            .attribute("index_shift")
-            .map(|s| sdf_parse_number(s, node))
-            .transpose()?;
+        let schedule_index_shift: Option<u64> = sdf_parse_attribute(xml_sdf, node, "index_shift")?;
 
         let mut schedule = vec![];
 
@@ -291,7 +283,15 @@ impl Domains {
             )
         })?;
 
-        let duration_int = sdf_parse_number(duration_raw, node)?;
+        let duration_int = parse_number(duration_raw).map_err(|err| {
+            format!(
+                "Error: failed to parse integer '{}' on element '{}': {}: {}",
+                duration_raw,
+                node.tag_name(),
+                err,
+                loc_string(xml_sdf, node.range().start),
+            )
+        })?;
         let duration = NonZero::new(duration_int).ok_or_else(|| {
             value_error(
                 xml_sdf,

tool/microkit/src/sdf/iommu.rs

@@ -6,11 +6,13 @@
 
 use std::collections::BTreeSet;
 use std::fmt;
-use std::str::FromStr;
 
 use super::memory_region::SysIOMap;
 use super::pci::{PciDevice, PciDeviceParseError};
-use super::util::{check_attributes, checked_lookup, loc_string, sdf_parse_number, value_error};
+use super::util::{
+    check_attributes, checked_lookup, loc_string, sdf_parse_required_attribute, value_error,
+    ParseableAttribute,
+};
 use super::{SdfNode, SystemDescriptionFile};
 
 use crate::{sel4::Arch, Config};
@@ -41,7 +43,7 @@ impl IommuDeviceIdentifier {
         s: &str,
     ) -> Result<Self, IommuDeviceIdentifierParseError> {
         match config.arch {
-            Arch::X86_64 => PciDevice::from_str(s)
+            Arch::X86_64 => PciDevice::parse(s)
                 .map(IommuDeviceIdentifier::X86Pci)
                 .map_err(IommuDeviceIdentifierParseError::Pci),
             Arch::Aarch64 | Arch::Riscv64 => Err(IommuDeviceIdentifierParseError::UnsupportedArch(
@@ -104,8 +106,8 @@ impl IOAddressSpace {
         // http://www.intel.com/content/dam/www/public/us/en/documents/product-specifications/vt-directed-io-spec.pdf
         let domain_id = match config.arch {
             Arch::X86_64 => {
-                let domain_id =
-                    sdf_parse_number(checked_lookup(xml_sdf, node, "domain_id")?, node)?;
+                let domain_id = sdf_parse_required_attribute(xml_sdf, node, "domain_id")?;
+
                 if !domain_ids.insert(domain_id) {
                     return Err(value_error(
                         xml_sdf,

tool/microkit/src/sdf/memory_region.rs

@@ -13,12 +13,14 @@ use sel4_capdl_initializer_types::FillEntryContentBootInfoId;
 
 use super::iommu::IommuDeviceIdentifier;
 use super::util::location_suffix_format;
-use super::util::{check_attributes, checked_lookup, sdf_parse_number, value_error};
+use super::util::{
+    check_attributes, checked_lookup, sdf_parse_attribute, sdf_parse_required_attribute,
+    value_error,
+};
 use super::{SdfLocation, SdfNode, SystemDescriptionFile};
 
 use crate::util::get_full_path;
 use crate::util::round_up;
-use crate::util::str_to_bool;
 use crate::{Config, PageSize};
 
 #[repr(u8)]
@@ -269,7 +271,7 @@ impl SysMap {
         check_attributes(xml_sdf, node, &attrs)?;
 
         let mr = checked_lookup(xml_sdf, node, "mr")?.to_string();
-        let vaddr = sdf_parse_number(checked_lookup(xml_sdf, node, "vaddr")?, node)?;
+        let vaddr: u64 = sdf_parse_required_attribute(xml_sdf, node, "vaddr")?;
 
         if vaddr >= max_vaddr {
             return Err(value_error(
@@ -304,21 +306,9 @@ impl SysMap {
             ));
         }
 
-        let cached = if let Some(xml_cached) = node.attribute("cached") {
-            match str_to_bool(xml_cached) {
-                Some(val) => val,
-                None => {
-                    return Err(value_error(
-                        xml_sdf,
-                        node,
-                        "cached must be 'true' or 'false'".to_string(),
-                    ))
-                }
-            }
-        } else {
+        let cached = sdf_parse_attribute(xml_sdf, node, "cached")?
             // Default to cached
-            true
-        };
+            .unwrap_or(true);
 
         Ok(SysMap {
             mr,
@@ -344,7 +334,7 @@ impl SysIOMap {
         check_attributes(xml_sdf, node, &attrs)?;
 
         let mr = checked_lookup(xml_sdf, node, "mr")?.to_string();
-        let iovaddr = sdf_parse_number(checked_lookup(xml_sdf, node, "iovaddr")?, node)?;
+        let iovaddr = sdf_parse_required_attribute(xml_sdf, node, "iovaddr")?;
 
         if iovaddr > x86_io_address_space::CAPDL_MAX_IOVA {
             return Err(value_error(
@@ -394,11 +384,8 @@ impl SysMemoryRegion {
         prefill_bootinfo_maybe: Option<FillEntryContentBootInfoId>,
         page_size: u64,
     ) -> Result<u64, String> {
-        match checked_lookup(xml_sdf, node, "size") {
-            Ok(size_str) => {
-                // Size explicitly specified
-                let size_parsed = sdf_parse_number(size_str, node)?;
-
+        match sdf_parse_attribute::<u64>(xml_sdf, node, "size")? {
+            Some(size_parsed) => {
                 if !size_parsed.is_multiple_of(page_size) {
                     return Err(value_error(
                         xml_sdf,
@@ -427,7 +414,7 @@ impl SysMemoryRegion {
                 }
             }
 
-            Err(_) => {
+            None => {
                 if prefill_bootinfo_maybe.is_some() {
                     Ok(page_size)
                 } else {
@@ -468,9 +455,9 @@ impl SysMemoryRegion {
         let name = checked_lookup(xml_sdf, node, "name")?;
 
         let mut page_size_specified_by_user = false;
-        let page_size = if let Some(xml_page_size) = node.attribute("page_size") {
+        let page_size = if let Some(page_size) = sdf_parse_attribute(xml_sdf, node, "page_size")? {
             page_size_specified_by_user = true;
-            sdf_parse_number(xml_page_size, node)?
+            page_size
         } else {
             config.page_sizes()[0]
         };
@@ -558,12 +545,10 @@ impl SysMemoryRegion {
             page_size,
         )?;
 
-        let phys_addr = if let Some(xml_phys_addr) = node.attribute("phys_addr") {
-            SysMemoryRegionPaddr::Specified(sdf_parse_number(xml_phys_addr, node)?)
-        } else {
+        let phys_addr = sdf_parse_attribute(xml_sdf, node, "phys_addr")?
+            .map(SysMemoryRegionPaddr::Specified)
             // At this point it is unsure whether this MR is a subject of a setvar region_paddr.
-            SysMemoryRegionPaddr::Unspecified
-        };
+            .unwrap_or(SysMemoryRegionPaddr::Unspecified);
 
         if let SysMemoryRegionPaddr::Specified(sdf_paddr) = phys_addr {
             if !sdf_paddr.is_multiple_of(page_size) {

tool/microkit/src/sdf/pci.rs

@@ -6,10 +6,11 @@
 
 use std::fmt;
 use std::ops::Deref;
-use std::str::FromStr;
 
 use sel4_capdl_initializer_types::object;
 
+use super::util::ParseableAttribute;
+
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
 pub struct PciDevice(pub object::PCIDevice);
 
@@ -88,10 +89,14 @@ impl fmt::Display for PciDeviceParseError {
     }
 }
 
-impl FromStr for PciDevice {
+impl ParseableAttribute for PciDevice {
     type Err = PciDeviceParseError;
 
-    fn from_str(s: &str) -> Result<Self, Self::Err> {
+    fn type_name() -> &'static str {
+        "pci device"
+    }
+
+    fn parse(s: &str) -> Result<Self, Self::Err> {
         let (bus_str, device_function_str) =
             s.split_once(':').ok_or(PciDeviceParseError::Malformed)?;
         let (device_str, function_str) = device_function_str