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

[framework] Remove DHM module #138

Closed
wants to merge 1 commit into from

Conversation

valeriosetti
Copy link
Contributor

@valeriosetti valeriosetti commented Feb 12, 2025

Description

Helps resolving Mbed-TLS/mbedtls#9956.

PR checklist

@valeriosetti valeriosetti self-assigned this Feb 12, 2025
@valeriosetti valeriosetti added 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 priority-high High priority - will be reviewed soon size-xs Estimated task size: extra small (a few hours at most) labels Feb 12, 2025
@@ -20,7 +20,7 @@
from generate_test_code import parse_function_arguments, parse_function_code
from generate_test_code import parse_functions, END_HEADER_REGEX
from generate_test_code import END_SUITE_HELPERS_REGEX, escaped_split
from generate_test_code import parse_test_data, gen_dep_check
from generate_test_code import gen_dep_check
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this import supposeed to be here?

Copy link
Contributor

@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.

Please don't remove the unit tests for parse_test_data. The fact that they happen to mention DHM is not really important, this code isn't technically related to the DHM module.

We could rewrite the unit tests to use function names that still exist in TF-PSA-Crypto, but it's unimportant. We can leave test_generate_test_code.py alone.

"""
data = """
Diffie-Hellman full exchange #1
dhm_do_dhm:10:"23":10:"5"
Copy link
Contributor

Choose a reason for hiding this comment

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

These are unit tests for the test code generator. They only compare the source .data file with the Python data obtained after parsing. It doesn't matter whether the first field of the second line is an existing function in Mbed TLS.

I guess it would be marginally better to have some actual code from a .data file as a unit test here, but it doesn't really matter. We can keep what's there.

We do, however, need to keep having unit tests for parse_test_data.

@valeriosetti
Copy link
Contributor Author

Closing as the only change requested in the framework for Mbed-TLS/mbedtls#9972 has been nacked.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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 priority-high High priority - will be reviewed soon size-xs Estimated task size: extra small (a few hours at most)
Development

Successfully merging this pull request may close these issues.

3 participants