Skip to content

Dev#6

Merged
ZR233 merged 39 commits intomainfrom
dev
Feb 13, 2026
Merged

Dev#6
ZR233 merged 39 commits intomainfrom
dev

Conversation

@ZR233
Copy link
Member

@ZR233 ZR233 commented Feb 13, 2026

No description provided.

ZR233 added 30 commits February 10, 2026 10:29
- Deleted the `property.rs` module which provided functionality for parsing and accessing device tree properties.
- Removed tests related to device tree headers and memory reservations that relied on the deleted module.
- Updated `Cargo.toml` to reflect the removal of the module and incremented the version to 0.2.0.
- Added new constants for size calculations in FDT parsing (`U32_SIZE`, `MEM_RSV_ENTRY_SIZE`).
- Improved `Bytes` struct with additional documentation and examples for clarity.
- Refactored `Fdt` struct methods to utilize new utility functions for path splitting and address translation.
- Introduced `fmt_utils` module for shared formatting functions, improving code organization and readability.
- Updated `Property` formatting to handle various types more gracefully, including a new method for typed formatting.
- Enhanced error handling in iterators to allow for graceful degradation during parsing.
- Added comprehensive documentation to several structs and methods to improve maintainability and usability.
- Removed obsolete node iteration and memory handling code from `node_iter` module.
- Introduced `NodeView` and `NodeViewMut` for safe, typed access to device tree nodes.
- Implemented visitor traits (`Visit` and `VisitMut`) for traversing device tree nodes.
- Updated tests to validate new node iteration and classification logic.
- Enhanced path lookup functionality to ensure correct resolution of node paths.
- Added `NodeGeneric` and `NodeGenericMut` for generic node views.
- Introduced `IntcNodeView` and `IntcNodeViewMut` for interrupt controller nodes.
- Created `MemoryNodeView` and `MemoryNodeViewMut` for memory nodes, including methods for parsing memory regions.
- Refactored `NodeView` and `NodeType` to support new specialized views.
- Removed the visitor traits and related implementations for traversing device tree nodes.
- Updated tests to reflect changes in node view access and removed obsolete visitor tests.
…ewMut and MemoryNodeViewMut; adjust visibility of add_child method in NodeGenericMut
Copilot AI review requested due to automatic review settings February 13, 2026 16:04
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR restructures the workspace around fdt-raw and fdt-edit, removing legacy crates (fdt-parser, dtb-tool) while adding new parsing/formatting utilities in fdt-raw and expanding fdt-edit’s arena-based editing + encoder test coverage.

Changes:

  • Add node path tracking to fdt-raw iteration (NodeBase::path() + iterator-maintained path stack) and add batch address translation tests.
  • Refactor/extend fdt-raw formatting and parsing utilities (property Display, alignment constants, UTF-8 error variant, new fmt_utils).
  • Rework fdt-edit tests and node “view” specializations; add encoding + rebuild round-trip tests; bump crate/workspace versions and dependencies; remove fdt-parser/dtb-tool.

Reviewed changes

Copilot reviewed 81 out of 82 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
fdt-raw/tests/ranges.rs Adds batch address translation test coverage.
fdt-raw/tests/node.rs Adds node path and child-lookup tests; tweaks memory size check logging.
fdt-raw/src/node/prop/reg.rs Expands docs and makes RegInfo::new const.
fdt-raw/src/node/prop/mod.rs Refactors Property formatting and uses shared U32_SIZE alignment constant.
fdt-raw/src/node/mod.rs Introduces stored path components and NodeBase::path(); uses shared indent helper.
fdt-raw/src/lib.rs Registers new fmt_utils module.
fdt-raw/src/iter.rs Adds iterator path stack maintenance for building node paths.
fdt-raw/src/header.rs Centralizes alignment requirement via Header::alignment().
fdt-raw/src/fmt_utils.rs New indentation helper module for formatting output.
fdt-raw/src/define.rs Replaces custom UTF-8 mapping with Utf8Error(#[from] Utf8Error).
fdt-raw/src/data.rs Adds U32_SIZE/MEM_RSV_ENTRY_SIZE constants and doc improvements; uses U32_SIZE in readers/iters.
fdt-raw/Cargo.toml Version bump to 0.2.0; dev-dep uses workspace.
Cargo.toml Workspace trims members and switches resolver to "3"; adds workspace dependency for dtb-file.
fdt-parser/tests/no_mem.rs Deleted (legacy crate removal).
fdt-parser/tests/head.rs Deleted (legacy crate removal).
fdt-parser/src/property.rs Deleted (legacy crate removal).
fdt-parser/src/lib.rs Deleted (legacy crate removal).
fdt-parser/src/header.rs Deleted (legacy crate removal).
fdt-parser/src/define.rs Deleted (legacy crate removal).
fdt-parser/src/data.rs Deleted (legacy crate removal).
fdt-parser/src/cache/node/mod.rs Deleted (legacy crate removal).
fdt-parser/src/cache/node/memory.rs Deleted (legacy crate removal).
fdt-parser/src/cache/node/interrupt_controller.rs Deleted (legacy crate removal).
fdt-parser/src/cache/node/clock.rs Deleted (legacy crate removal).
fdt-parser/src/cache/node/chosen.rs Deleted (legacy crate removal).
fdt-parser/src/cache/mod.rs Deleted (legacy crate removal).
fdt-parser/src/cache/fdt.rs Deleted (legacy crate removal).
fdt-parser/src/base/node/memory.rs Deleted (legacy crate removal).
fdt-parser/src/base/node/interrupt_controller.rs Deleted (legacy crate removal).
fdt-parser/src/base/node/chosen.rs Deleted (legacy crate removal).
fdt-parser/src/base/mod.rs Deleted (legacy crate removal).
fdt-parser/examples/phytium.rs Deleted (legacy crate removal).
fdt-parser/examples/bcm2711.rs Deleted (legacy crate removal).
fdt-parser/README.md Deleted (legacy crate removal).
fdt-parser/LICENSE Deleted (legacy crate removal).
fdt-parser/Cargo.toml Deleted (legacy crate removal).
fdt-parser/.gitignore Deleted (legacy crate removal).
dtb-tool/src/main.rs Deleted (legacy tool removal).
dtb-tool/Cargo.toml Deleted (legacy tool removal).
fdt-edit/tests/remove_node.rs Deleted (test suite reorg).
fdt-edit/tests/rebuild.rs New DTB rebuild test via dtc+diff.
fdt-edit/tests/range.rs Updates reg translation tests to new APIs.
fdt-edit/tests/pci.rs Updates PCI tests to new NodeType/view APIs.
fdt-edit/tests/memory.rs Deleted (replaced by new test structure).
fdt-edit/tests/irq.rs Deleted (replaced by new test structure).
fdt-edit/tests/find2.rs Deleted (replaced by new test structure).
fdt-edit/tests/fdt.rs New tests for iteration, classification, path lookup, display.
fdt-edit/tests/encode.rs New encoder + round-trip tests.
fdt-edit/tests/edit.rs Deleted (replaced by rebuild/encode tests).
fdt-edit/tests/display_debug.rs Deleted (replaced by display tests elsewhere).
fdt-edit/tests/clock.rs Updates clock tests to new NodeType/view APIs.
fdt-edit/src/prop/mod.rs Switches to crate-level Reader import usage.
fdt-edit/src/node/view/memory.rs New memory node view specialization.
fdt-edit/src/node/view/intc.rs New interrupt controller node view specialization.
fdt-edit/src/node/view/generic.rs New generic node view wrappers (mutable + immutable).
fdt-edit/src/node/view/clock.rs New clock node view specialization and clock-ref types.
fdt-edit/src/node/memory.rs Deleted (replaced by view-based design).
fdt-edit/src/node/iter.rs Deleted (replaced by new iteration model).
fdt-edit/src/node/interrupt_controller.rs Deleted (replaced by view-based design).
fdt-edit/src/node/gerneric.rs Deleted (replaced by view-based design).
fdt-edit/src/node/clock.rs Deleted (replaced by view-based design).
fdt-edit/src/lib.rs Simplifies exports; re-exports fdt_raw types; introduces NodeId.
fdt-edit/src/encode.rs Updates encoder to work with arena-based node IDs.
fdt-edit/src/ctx.rs Deleted (context approach removed).
fdt-edit/examples/fdt_debug_demo.rs Deleted (example cleanup).
fdt-edit/README.md Deleted (package metadata cleanup).
fdt-edit/Cargo.toml Version bump to 0.2.0; dependency/version alignment with fdt-raw; workspace dev-dep.
example_all_nodes.rs Deleted (example cleanup).
CLAUDE.md Deleted (repo meta cleanup).
.claude/settings.local.json Updates tool allowlist configuration.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +157 to +167
pub fn path(&self) -> heapless::String<256> {
let mut result = heapless::String::new();
if self.path_components.is_empty() {
let _ = result.push('/');
return result;
}
for component in &self.path_components {
let _ = result.push('/');
let _ = result.push_str(component);
}
result
Copy link

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NodeBase::path() ignores the results of push/push_str into a fixed-capacity heapless::String<256>. If the path exceeds 256 bytes, the path will be silently truncated. Consider returning a Result, increasing capacity, or otherwise surfacing overflow so callers/tests don't get incorrect paths.

Copilot uses AI. Check for mistakes.
Comment on lines 168 to 175
// Has child nodes, update reader position
self.reader = node_iter.reader().clone();
// Push current node name onto path stack
if !node.name().is_empty() {
let _ = self.path_stack.push(node.name());
}
// Increase level (node has children)
self.level += 1;
Copy link

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

path_stack has a fixed capacity (16) and the push result is ignored. If the node depth exceeds this, the push will fail silently while pop() will still run on EndNode, which can lead to incorrect path context for subsequent nodes. Consider handling the push error (e.g., stop iteration with a clear error) or increasing capacity to cover realistic DTB depths.

Copilot uses AI. Check for mistakes.
Comment on lines +68 to +80
// 3. 保存到 /tmp
let tmp_dir = "/tmp/fdt_rebuild_test";
fs::create_dir_all(tmp_dir).unwrap_or_else(|_| panic!("Failed to create tmp dir"));

let orig_dtb_path = format!("{}/{}.orig.dtb", tmp_dir, case.name);
let enc_dtb_path = format!("{}/{}.enc.dtb", tmp_dir, case.name);
let orig_dts_path = format!("{}/{}.orig.dts", tmp_dir, case.name);
let enc_dts_path = format!("{}/{}.enc.dts", tmp_dir, case.name);

fs::write(&orig_dtb_path, &raw_data[..])
.unwrap_or_else(|_| panic!("Failed to write {}", orig_dtb_path));
fs::write(&enc_dtb_path, &encoded[..])
.unwrap_or_else(|_| panic!("Failed to write {}", enc_dtb_path));
Copy link

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test writes to a fixed directory (/tmp/fdt_rebuild_test). If tests run concurrently (or multiple CI jobs share a runner), files from different cases/runs can collide and cause spurious failures. Consider using a unique temp directory per test run/case (e.g., tempfile::tempdir() or include PID/random suffix) and cleaning it up.

Copilot uses AI. Check for mistakes.
Comment on lines +82 to +95
// 4. 使用 dtc 反编译为 DTS
let dtc_status_orig = Command::new("dtc")
.arg("-I")
.arg("dtb")
.arg("-O")
.arg("dts")
.arg("-o")
.arg(&orig_dts_path)
.arg(&orig_dtb_path)
.status()
.unwrap_or_else(|_| panic!("Failed to run dtc on original DTB"));

assert!(dtc_status_orig.success(), "dtc failed on original DTB");

Copy link

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test unconditionally requires external tools (dtc and diff) and will fail hard if they aren’t installed. Consider detecting tool availability (e.g., Command::new("dtc").arg("--version")) and skipping the test with a clear message when missing, to keep cargo test usable on Linux environments without these dependencies.

Copilot uses AI. Check for mistakes.
Comment on lines 29 to 61
let root_id = fdt.root_id();
let node = fdt.node_mut(root_id).unwrap();
node.set_property(crate::Property::new("#address-cells", vec![0x00, 0x00, 0x00, 0x02]));
node.set_property(crate::Property::new("#size-cells", vec![0x00, 0x00, 0x00, 0x01]));
node.set_property(crate::Property::new("model", {
let mut v = b"Test Device".to_vec();
v.push(0);
v
}));

let encoded = fdt.encode();

// 解析并验证
let parsed = Fdt::from_bytes(&encoded).unwrap();
let root = parsed.get_by_path("/").unwrap();
let node_ref = root.as_node();

// 验证属性
assert_eq!(node_ref.address_cells(), Some(2));
assert_eq!(node_ref.size_cells(), Some(1));
assert_eq!(node_ref.get_property("model").unwrap().as_str(), Some("Test Device"));
}

/// 测试带有子节点的 FDT 编码
#[test]
fn test_encode_with_children() {
let mut fdt = Fdt::new();

// 添加子节点
let root_id = fdt.root_id();
let mut soc = crate::Node::new("soc");
soc.set_property(crate::Property::new("#address-cells", vec![0x00, 0x00, 0x00, 0x02]));
soc.set_property(crate::Property::new("#size-cells", vec![0x00, 0x00, 0x00, 0x02]));
Copy link

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This integration test uses crate::Property/crate::Node, but in tests/*.rs (integration tests) crate refers to the test crate, not fdt_edit. This will fail to compile. Use Property::new(...) / Node::new(...) (already imported via use fdt_edit::*;) or qualify with fdt_edit::Property / fdt_edit::Node.

Copilot uses AI. Check for mistakes.
Comment on lines +9 to +54
#[cfg(target_os = "linux")]
use dtb_file::*;
#[cfg(target_os = "linux")]
use fdt_edit::*;
#[cfg(target_os = "linux")]
use std::fs;
#[cfg(target_os = "linux")]
use std::process::Command;

/// 测试用例
struct DtbTestCase {
name: &'static str,
loader: fn() -> Align4Vec,
}

/// 所有测试用例
const TEST_CASES: &[DtbTestCase] = &[
DtbTestCase {
name: "qemu",
loader: || fdt_qemu(),
},
DtbTestCase {
name: "pi_4b",
loader: || fdt_rpi_4b(),
},
DtbTestCase {
name: "phytium",
loader: || fdt_phytium(),
},
DtbTestCase {
name: "rk3568",
loader: || fdt_3568(),
},
DtbTestCase {
name: "reserve",
loader: || fdt_reserve(),
},
];

/// 主测试函数:遍历所有测试用例
#[test]
fn test_rebuild_all() {
for case in TEST_CASES {
test_rebuild_single(case);
}
}
Copy link

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file only cfgs the imports, but the rest of the code (e.g., Align4Vec, Fdt, fs, Command, the tests, and constants) is unconditional. On non-Linux targets this will not compile. Gate the whole file (or at least DtbTestCase, TEST_CASES, and the tests) with #[cfg(target_os = "linux")], or provide a stub #[test] for other targets that is ignored/skipped.

Copilot uses AI. Check for mistakes.
Comment on lines +340 to +345
// Build path components: parent path + current node name
let mut path_components = parent_path.clone();
if !name.is_empty() {
let _ = path_components.push(name);
}

Copy link

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

path_components has a fixed capacity (16), but the push result is ignored. If a DTB contains a deeper path, the push will fail and the node path will be silently truncated, producing incorrect NodeBase::path() values. Consider increasing the capacity, returning an error when the path stack overflows, or using a fallible path builder so failures are detectable.

Copilot uses AI. Check for mistakes.
Copilot AI review requested due to automatic review settings February 13, 2026 16:34
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 82 out of 83 changed files in this pull request and generated 3 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +157 to +166
pub fn path(&self) -> heapless::String<256> {
let mut result = heapless::String::new();
if self.path_components.is_empty() {
let _ = result.push('/');
return result;
}
for component in &self.path_components {
let _ = result.push('/');
let _ = result.push_str(component);
}
Copy link

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NodeBase::path() builds a heapless::String<256> but ignores the Result from push/push_str. If the path exceeds 256 bytes, the method will silently truncate, producing incorrect paths and potentially causing wrong lookups/debug output. Consider returning a Result (or at least debug_assert!/expect) on overflow, and/or increasing capacity or providing an allocating String alternative behind a std/alloc feature.

Copilot uses AI. Check for mistakes.
Comment on lines +31 to +39
node.set_property(crate::Property::new(
"#address-cells",
vec![0x00, 0x00, 0x00, 0x02],
));
node.set_property(crate::Property::new(
"#size-cells",
vec![0x00, 0x00, 0x00, 0x01],
));
node.set_property(crate::Property::new("model", {
Copy link

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These integration tests use crate::Property/crate::Node, but in tests/*.rs files crate refers to the test crate, not fdt_edit. This will fail to compile. Use Property::new(...) / Node::new(...) (already in scope via use fdt_edit::*;) or prefix with fdt_edit::Property / fdt_edit::Node.

Copilot uses AI. Check for mistakes.
Comment on lines +68 to +85
let mut soc = crate::Node::new("soc");
soc.set_property(crate::Property::new(
"#address-cells",
vec![0x00, 0x00, 0x00, 0x02],
));
soc.set_property(crate::Property::new(
"#size-cells",
vec![0x00, 0x00, 0x00, 0x02],
));
let soc_id = fdt.add_node(root_id, soc);

let mut uart = crate::Node::new("uart@1000");
uart.set_property(crate::Property::new("reg", {
let v = vec![0x00, 0x00, 0x10, 0x00, 0x00, 0x00, 0x10, 0x00];
v
}));
uart.set_property(crate::Property::new("compatible", {
let mut v = b"test,uart".to_vec();
Copy link

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same issue: crate::Node::new / crate::Property::new in an integration test refers to the test crate, not fdt_edit, so this won’t compile. Replace with Node::new / Property::new (or fdt_edit::...).

Copilot uses AI. Check for mistakes.
@ZR233 ZR233 merged commit ab4a969 into main Feb 13, 2026
4 checks passed
@ZR233 ZR233 deleted the dev branch February 13, 2026 16:39
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