Skip to content

Change the compatibility API to inline functions #365

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

Merged
merged 1 commit into from
Feb 18, 2020

Conversation

soby-mathew
Copy link
Contributor

@soby-mathew soby-mathew commented Feb 10, 2020

This patch changes the compatibility API defined in crypto_compat.h
to static inline functions as the previous macro definitions were
causing issues for the C pre-processor when included in projects
which need to redefine the PSA function names. Making it static
inline function solves this problem neatly and also modern compilers
do a good job at inlining the function which makes the need for making
it a macro redundant.

Signed-off-by: Soby Mathew [email protected]

@gilles-peskine-arm
Copy link
Collaborator

Thanks Soby. Making the old function names be static inline functions rather than macros is ok. However, you'll need to change several things to pass the CI. I think the functions should return psa_status_t: mbedtls_deprecated_psa_status_t is a hack to get a deprecation warning via macro usage. Instead, use MBEDTLS_PSA_DEPRECATED at the beginning of the function declaration.

Please ensure that the CI passes. In particular, run

tests/scripts/check-files.py
tests/scripts/check-names.sh
tests/scripts/all.sh build_deprecated

@gilles-peskine-arm gilles-peskine-arm added enhancement New feature or request needs: ci Needs a passing full CI run needs: work The pull request needs rework before it can be merged. labels Feb 10, 2020
This patch changes the compatibility API defined in crypto_compat.h
to static inline functions as the previous macro definitions were
causing issues for the C pre-processor when included in projects
which need to redefine the PSA function names. Making it static
inline function solves this problem neatly and also modern compilers
do a good job at inlining the function which makes the need for making
it a macro redundant.

Signed-off-by: Soby Mathew <[email protected]>
@soby-mathew
Copy link
Contributor Author

Thanks Gilles. I have updated with new patch.

@gilles-peskine-arm
Copy link
Collaborator

CI is failing only on a known unrelated issue on Mbed OS, so that's as good as a pass.

@gilles-peskine-arm gilles-peskine-arm added needs: review The pull request is ready for review. This generally means that it has no known issues. and removed needs: ci Needs a passing full CI run needs: work The pull request needs rework before it can be merged. labels Feb 10, 2020
Copy link
Collaborator

@gilles-peskine-arm gilles-peskine-arm left a comment

Choose a reason for hiding this comment

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

Thank you. I'm trying to remember why I didn't make these inline functions originally. Since I didn't give any reason in the commit message, and I can't come up with a reason now, that must have been an oversight.

Copy link
Contributor

@mpg mpg left a comment

Choose a reason for hiding this comment

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

LGTM

@mpg mpg removed the needs: review The pull request is ready for review. This generally means that it has no known issues. label Feb 18, 2020
@mpg mpg merged commit f8b9329 into ARMmbed:development Feb 18, 2020
gilles-peskine-arm pushed a commit to gilles-peskine-arm/mbed-crypto that referenced this pull request Mar 23, 2020
* ARMmbed#365 Change PSA compatibility API to inline functions
* ARMmbed#367 Fix pk_parse_key()'s use of rsa_complete()
* ARMmbed#370 Bump version to Mbed TLS 2.21.0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants