Skip to content

hashcrypt: minimize panic code paths#528

Merged
jerrysxie merged 6 commits intoOpenDevicePartnership:mainfrom
jerrysxie:depanic-hashcrypt
Dec 19, 2025
Merged

hashcrypt: minimize panic code paths#528
jerrysxie merged 6 commits intoOpenDevicePartnership:mainfrom
jerrysxie:depanic-hashcrypt

Conversation

@jerrysxie
Copy link
Contributor

@jerrysxie jerrysxie commented Dec 16, 2025

This pull request enhances error handling in the hashcrypt hasher implementation by introducing a unified Error type and updating relevant methods to return Result instead of panicking. This makes the code more robust, especially for invalid input scenarios, and improves its safety and reliability in both blocking and asynchronous hashing modes.

Error handling improvements:

  • Introduced a new Error enum in src/hashcrypt/mod.rs to represent unsupported configurations and updated code to use this error type instead of panicking.
  • Updated internal helper methods in Hasher (such as init_final_data, init_final_block, and init_final_len) to return Result<(), Error> and propagate errors instead of panicking on invalid buffer access.

Blocking hasher API changes:

  • Changed public and internal methods in the blocking Hasher implementation (submit_blocks, finalize, hash) to return Result and handle errors gracefully, replacing panics with error returns for invalid input lengths.

Async hasher API changes:

  • Updated async Hasher methods (transfer, submit_blocks, finalize, hash) to return Result and propagate errors, ensuring safe handling of invalid conditions such as missing DMA channels or incorrect data lengths. [1] [2] [3]

General code safety improvements:

  • Added assertions and explicit error returns to ensure buffer sizes and data lengths are always valid, replacing unchecked unwraps and panics with error handling. [1] [2]

@jerrysxie jerrysxie self-assigned this Dec 16, 2025
Copilot AI review requested due to automatic review settings December 16, 2025 21:35
@jerrysxie jerrysxie added the BREAKING CHANGE PR causes a breaking change label Dec 16, 2025
@jerrysxie jerrysxie added enhancement New feature or request core hal MCU core HAL functionality labels Dec 16, 2025
Copy link
Contributor

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 refactors the hashcrypt module to replace panic-based error handling with Result-based error handling, introducing a new Error enum and updating all public APIs to return Result<(), Error>.

  • Introduces a new Error enum with an UnsupportedConfiguration variant
  • Converts internal helper methods to use bounds checking with error returns instead of direct indexing
  • Updates all public methods (submit_blocks, finalize, hash) for both blocking and async variants to return Results

Reviewed changes

Copilot reviewed 2 out of 4 changed files in this pull request and generated 5 comments.

File Description
src/hashcrypt/mod.rs Adds the new Error enum with UnsupportedConfiguration variant and defmt support
src/hashcrypt/hasher.rs Converts internal methods and public APIs to return Results, replacing panics with error handling (except one remaining panic in async transfer)
examples/rt685s-evk/src/bin/sha256.rs Updates example to handle Results with .unwrap() calls on hash operations
examples/rt685s-evk/src/bin/sha256-async.rs Updates async example to handle Results with .unwrap() calls on hash operations

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

* Fix format

* Replace panic! with error return
@jerrysxie jerrysxie marked this pull request as ready for review December 17, 2025 00:40
@jerrysxie jerrysxie requested a review from a team as a code owner December 17, 2025 00:40
Copy link
Contributor

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 2 out of 4 changed files in this pull request and generated 3 comments.


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

kurtjd
kurtjd previously approved these changes Dec 17, 2025
williampMSFT
williampMSFT previously approved these changes Dec 17, 2025
Copy link
Contributor

@williampMSFT williampMSFT left a comment

Choose a reason for hiding this comment

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

:shipit:

@jerrysxie jerrysxie dismissed stale reviews from williampMSFT and kurtjd via 7cf149b December 17, 2025 19:44
@williampMSFT williampMSFT self-requested a review December 17, 2025 19:47
williampMSFT
williampMSFT previously approved these changes Dec 17, 2025
kurtjd
kurtjd previously approved these changes Dec 17, 2025
Copilot AI review requested due to automatic review settings December 17, 2025 23:17
@jerrysxie jerrysxie dismissed stale reviews from kurtjd and williampMSFT via ab84d4a December 17, 2025 23:17
Copy link
Contributor

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 2 out of 4 changed files in this pull request and generated 4 comments.


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

@jerrysxie
Copy link
Contributor Author

@jerrysxie jerrysxie enabled auto-merge (squash) December 18, 2025 21:52
@jerrysxie jerrysxie disabled auto-merge December 18, 2025 21:53
@jerrysxie jerrysxie enabled auto-merge (squash) December 18, 2025 22:01
Copilot AI review requested due to automatic review settings December 18, 2025 22:01
Copy link
Contributor

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 2 out of 4 changed files in this pull request and generated 3 comments.


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

@jerrysxie jerrysxie merged commit 92c81e7 into OpenDevicePartnership:main Dec 19, 2025
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

BREAKING CHANGE PR causes a breaking change core hal MCU core HAL functionality enhancement New feature or request

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants