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

core: Improve ID hex formatting #186

Merged
merged 7 commits into from
Dec 17, 2024
Merged

core: Improve ID hex formatting #186

merged 7 commits into from
Dec 17, 2024

Conversation

robin-nitrokey
Copy link
Member

This patch tries to improve the confusing situation with ID hex formatting:

  • The original Id::hex implementation (also used in Id::hex_path) skipped inner zero bytes so that different numbers are mapped to the same hex string. We need to keep this behavior for compatibility. But to indicate this flaw and avoid usage in new code, this PR removes Id::hex and renames Id::hex_path to Id::legacy_hex_path. Id::hex_path is kept as a deprecated alias for easier migration.
  • There already is the Id::hex_clean function that correctly formats numbers with inner zero bytes. This PR also adds Id::clean_hex_path that can be used for paths in new code.
  • The old Id::hex_clean function skipped all zero bytes, including the trailing zero. As it is currently only used in the Debug implementation, this PR changes it to format 0 as "00".
  • This PR also adds unit tests for these three functions and removes unnecessary buffer allocations in the implementation.
  • As a side effect, this allows us to drop the public heapless dependency in trussed-core. Not really important right now, but will be useful once we can upgrade heapless-bytes.

@robin-nitrokey robin-nitrokey merged commit eadd27c into main Dec 17, 2024
2 checks passed
@robin-nitrokey robin-nitrokey deleted the hex-id branch December 17, 2024 09:18
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.

2 participants