Relax p1 to use it as an optional feature bitmap #133
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
src/boilerplate/dispatcher.c
Outdated
| @@ -127,7 +127,7 @@ void apdu_dispatcher(command_descriptor_t const cmd_descriptors[], | |||
|
|
|||
| G_dispatcher_context.read_buffer = buffer_create(cmd->data, cmd->lc); | |||
|
|
|||
| if (cmd->p1 != 0 || cmd->p2 > 1) { | |||
| if (cmd->p2 > 1) { | |||
There was a problem hiding this comment.
nit : If this 1 value corresponds to the protocol version maybe a comment or a define could be nice ? :)
There was a problem hiding this comment.
Good point, added a constant in 4b199ce (and rebase to the current develop).
sgliner-ledger
approved these changes
Mar 31, 2023
|
SonarCloud Quality Gate failed. |
Codecov ReportPatch and project coverage have no change.
Additional details and impacted files@@ Coverage Diff @@
## develop #133 +/- ##
========================================
Coverage 83.96% 83.96%
========================================
Files 17 17
Lines 2189 2189
========================================
Hits 1838 1838
Misses 351 351
Flags with carried forward coverage won't be shown. Click here to find out more. Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
For some future use cases, it might be useful to have some additional optional features that can be activated by a client that supports them, but do not otherwise disrupt the normal flow of operations.
The
p1APDU field is currently unused and forced to be 0.By relaxing the requirement and ignoring the value of
p1we guarantee that such optional features can be added without breaking forward-compatibility or forcing a protocol version upgrade (which instead uses thep2field).I also added a test to make sure that a
p2value higher than the current protocol version is rejected; for the protocol version we prefer explicit opt-in, instead − it would generally be used for more drastic breaking changes to the protocol.