-
Notifications
You must be signed in to change notification settings - Fork 13
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
PSA_WANT_xxx is only meaningful when PSA crypto is enabled #58
PSA_WANT_xxx is only meaningful when PSA crypto is enabled #58
Conversation
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
ef7cd7b
to
239c0d8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@@ -80,7 +80,7 @@ def dependencies_of_setting(cfg: config_common.Config, | |||
be negated by prefixing them with '!'. This is the same syntax as a | |||
depends_on directive in test data. | |||
""" | |||
#pylint: disable=too-many-return-statements | |||
#pylint: disable=too-many-branches,too-many-return-statements |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nitpick: instead of silencing the warning, we could move the TLS setting processing to a separate function.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Fix a missing dependency in configuration symbol test enumeration:
PSA_WANT_xxx
is only meaningful when PSA crypto is enabled. This isn't needed in TF-PSA-Crypto where all crypto goes through PSA anyway, but it is relevant in 3.6 and harmless (as long as we keepMBEDTLS_PSA_CRYPTO_CLIENT
at least as an internal symbol) in TF-PSA-Crypto.