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

Unify mechanism traits #162

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

robin-nitrokey
Copy link
Member

This patch combines the operation traits that were previously used to call mechanism implementations into a single MechanismImpl trait. This has several advantages:

  • We can use a macro to implement the dispatch from the Mechanism enum, removing boilerplate code from the reply_to implementation.
  • To implement an operation for a mechanism, it is now sufficient to override the respective trait method. It is no longer necessary to also update reply_to.
  • The need to annotate all mechanism methods with #[inline(never)] to avoid producing a huge reply_to function (see the comment in mechanisms.rs) is reduced as we can just mark the methods generated by the macro as #[inline(never)].
  • This reduces the binary size required in the stable nitrokey-3-firmware by some kB.

This patch combines the operation traits that were previously used to
call mechanism implementations into a single MechanismImpl trait.  This
has several advantages:
- We can use a macro to implement the dispatch from the Mechanism enum,
  removing boilerplate code from the reply_to implementation.
- To implement an operation for a mechanism, it is now sufficient to
  override the respective trait method.  It is no longer necessary to
  also update reply_to.
- The need to annotate all mechanism methods with #[inline(never)] to
  avoid producing a huge reply_to function (see the comment in
  mechanisms.rs) is reduced as we can just mark the methods generated by
  the macro as #[inline(never)].
- This reduces the binary size required in the stable
  nitrokey-3-firmware by some kB.
@sosthene-nitrokey
Copy link
Contributor

I like that

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