Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Validate node and property names #33

Merged
merged 1 commit into from
Aug 25, 2021

Conversation

danielverkamp
Copy link
Collaborator

Add checks for the requirements in the devicetree specifications.

This does not check for any specific bus binding requirements for unit
addresses (as in "node-name@unit-address"); only the generic
requirements for all node names are enforced.

Fixes #32.

Signed-off-by: Daniel Verkamp dverkamp@chromium.org

@@ -255,6 +330,9 @@ impl FdtWriter {
/// `name` - name of the node; must not contain any NUL bytes.
pub fn begin_node(&mut self, name: &str) -> Result<FdtWriterNode> {
let name_cstr = CString::new(name).map_err(|_| Error::InvalidString)?;
if !node_name_valid(name) {
Copy link
Member

Choose a reason for hiding this comment

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

Can we add a comment here that in case the user uses unit-address, then they must provide a valid one according to the specification (and then add a link to the spec)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sure, added a comment to this effect.

assert!(!node_name_valid("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"));
assert!(!node_name_valid("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa@1234"));

// Name contains invalid characters.
Copy link
Member

Choose a reason for hiding this comment

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

Can we also add a test case with the invalid character /? Having that char in the name will split the node in 2 nodes, so I think we should check for it in unit tests as well.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good point - I added a test for a name containing "/".

Add checks for the requirements in the devicetree specifications.

This does not check for any specific bus binding requirements for unit
addresses (as in "node-name@unit-address"); only the generic
requirements for all node names are enforced.

Fixes rust-vmm#32.

Signed-off-by: Daniel Verkamp <dverkamp@chromium.org>
@lauralt lauralt merged commit 146f55c into rust-vmm:master Aug 25, 2021
@danielverkamp danielverkamp deleted the node-name-valid branch August 25, 2021 21: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.

begin_node does not enforce limits defined in the specification
3 participants