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

Backport 2.28: Don't restyle some PSA macros #6897

Conversation

gilles-peskine-arm
Copy link
Contributor

Trivial backport of #6878

Gatekeeper checklist

  • changelog not required (not user-visible)
  • backport of Don't restyle some PSA macros #6878
  • tests will be validated by all.sh test_psa_compliance passing on the preview branch once it's updated; check manually by running scripts/code_style.py && tests/scripts/all.sh test_psa_compliance)

Some preprocessor macro definitions must have a specific expansion so that
the same macro name can be defined in different products. The definition of
having the same expansion (per the C language specification) means the same
sequence of tokens, and also the same absence/presence of spacing between
tokens.

For PSA error code definitions, the specific expansion is mandated by the
PSA Status code specification and the PSA Crypto API specification. In
particular, there must not be a space between (psa_status_t) and the
numerical value (whereas K&R would put a space).

Signed-off-by: Gilles Peskine <[email protected]>
Some preprocessor macro definitions must have a specific expansion so that
the same macro name can be defined in different products. The definition of
having the same expansion (per the C language specification) means the same
sequence of tokens, and also the same absence/presence of spacing between
tokens.

Two macros are also defined in headers in the PSA Compliance test suite, so
the test suite would fail to build if we changed the definitions. Preserve
those definitions. Technically this is a bug in the test suite, since having
extra spaces (or even a completely different constant expression with the
same value) would still be compliant. Bug reported as
ARM-software/psa-arch-tests#337

Signed-off-by: Gilles Peskine <[email protected]>
@gilles-peskine-arm gilles-peskine-arm added needs-review Every commit must be reviewed by at least two team members, component-platform Portability layer and build scripts needs-ci Needs to pass CI tests needs-reviewer This PR needs someone to pick it up for review size-s Estimated task size: small (~2d) priority-high High priority - will be reviewed soon labels Jan 10, 2023
Copy link
Contributor

@davidhorstmann-arm davidhorstmann-arm left a comment

Choose a reason for hiding this comment

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

LGTM as a faithful backport

@davidhorstmann-arm davidhorstmann-arm added single-reviewer This PR qualifies for having only one reviewer and removed needs-review Every commit must be reviewed by at least two team members, needs-reviewer This PR needs someone to pick it up for review labels Jan 10, 2023
@davidhorstmann-arm
Copy link
Contributor

Marked as single reviewer since this is a trivial backport that only changes comments.

@davidhorstmann-arm davidhorstmann-arm added the approved Design and code approved - may be waiting for CI or backports label Jan 10, 2023
@gilles-peskine-arm gilles-peskine-arm removed the needs-ci Needs to pass CI tests label Jan 10, 2023
@gilles-peskine-arm gilles-peskine-arm merged commit 83fc93e into Mbed-TLS:mbedtls-2.28 Jan 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Design and code approved - may be waiting for CI or backports component-platform Portability layer and build scripts priority-high High priority - will be reviewed soon single-reviewer This PR qualifies for having only one reviewer size-s Estimated task size: small (~2d)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants