Skip to content

Add I2C and GPIO resource descriptors - #333

Open
dewyatt wants to merge 7 commits into
rust-osdev:mainfrom
dewyatt:dewyatt-add-serial-gpio-descriptors
Open

Add I2C and GPIO resource descriptors#333
dewyatt wants to merge 7 commits into
rust-osdev:mainfrom
dewyatt:dewyatt-add-serial-gpio-descriptors

Conversation

@dewyatt

@dewyatt dewyatt commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

No description provided.

@martin-hughes martin-hughes left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for this PR as well @dewyatt - it looks fundamentally sound to me.

Just a few nitpicks really, and a couple of questions for @IsaacWoods.

Comment thread src/aml/resource.rs Outdated
Comment thread src/aml/resource.rs
Comment thread src/aml/resource.rs
Comment thread src/aml/resource.rs Outdated
Comment thread src/aml/resource.rs
Comment thread src/aml/resource.rs Outdated
Comment thread src/aml/resource.rs Outdated
Comment thread src/aml/resource.rs

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@IsaacWoods should we be comparing the length of the descriptor buffer slices against the value of the length field contained within that slice?

I know there's no memory safety risk of skipping this check, it'd mostly be to avoid an out-of-bounds panic.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yes I think the ideal would be for parsing resource descriptors to be entirely panic free (I can't recall if that's the position we're in atm)

Comment thread src/aml/resource.rs
@IsaacWoods

IsaacWoods commented Sep 2, 2026

Copy link
Copy Markdown
Member

Thanks @dewyatt for the PR and to @martin-hughes for review. Very happy for this to be merged from my end when questions addressed :)

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.

3 participants