Skip to content

Conversation

@kurtjd
Copy link
Contributor

@kurtjd kurtjd commented Dec 19, 2025

This PR audits potential panics. Most were cases that are easily proven to never panic (assuming the assumptions in the code are held) and these assumptions are defined very close to the potential panic path, so I think it was safe to allow these instead of returning an error.

A refactor of the Args type may help eliminate the need for unwraps (see: #61).

Rdo::for_pdo now returns an Option<Self> and is public so a minor breaking change has been introduced.

Resolves #59

@kurtjd kurtjd self-assigned this Dec 19, 2025
@kurtjd kurtjd added the enhancement New feature or request label Dec 19, 2025
@kurtjd kurtjd requested a review from a team as a code owner December 19, 2025 21:41
@kurtjd kurtjd moved this to In review in Embedded Controller Dec 19, 2025
@kurtjd kurtjd force-pushed the audit-panics branch 2 times, most recently from e86645b to 5e04e7d Compare December 19, 2025 22:19
@jerrysxie
Copy link
Contributor

@kurtjd @asasine It seems like once this is merged, we will need a corresponding changes in embedded-services as well.

@kurtjd kurtjd changed the title Audit panics embedded-usb-pd: Audit panics Dec 23, 2025
jerrysxie
jerrysxie previously approved these changes Dec 23, 2025
Copy link
Contributor

@jerrysxie jerrysxie left a comment

Choose a reason for hiding this comment

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

@kurtjd Don't forget about the corresponding changes in embedded-services.

@kurtjd
Copy link
Contributor Author

kurtjd commented Dec 23, 2025

@kurtjd Don't forget about the corresponding changes in embedded-services.

Will announce this on Zulip and submit a PR for embedded-services.

@kurtjd kurtjd merged commit 9a42f07 into OpenDevicePartnership:main Dec 23, 2025
10 checks passed
@github-project-automation github-project-automation bot moved this from In review to Done in Embedded Controller Dec 23, 2025
@kurtjd
Copy link
Contributor Author

kurtjd commented Dec 23, 2025

@kurtjd Don't forget about the corresponding changes in embedded-services.

Will announce this on Zulip and submit a PR for embedded-services.

embedded-services PR: OpenDevicePartnership/embedded-services#664
Zulip announcement: https://opendevicepartnership.zulipchat.com/#narrow/channel/495405-embedded-controller/topic/embedded-usb-pd.20breaking.20change/with/565213312

jerrysxie pushed a commit to OpenDevicePartnership/embedded-services that referenced this pull request Dec 24, 2025
Updates repo to the latest de-panicked `embedded-usb-pd`:
OpenDevicePartnership/embedded-usb-pd#60
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

BREAKING CHANGE enhancement New feature or request

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

embedded-usb-pd: Audit for panics

3 participants