mbedtls icon indicating copy to clipboard operation
mbedtls copied to clipboard

Backport 2.28: Report configuration settings in the outcome file

Open gilles-peskine-arm opened this issue 1 year ago • 3 comments
trafficstars

Backport of https://github.com/Mbed-TLS/mbedtls/pull/9172 and its framework companion https://github.com/Mbed-TLS/mbedtls-framework/pull/28.

I followed the same commit structure for the common content, but several things are different in 2.28.

  • Minor fixes from the main PR.
  • Test code and manually written test cases.
  • Commits from the framework PR to create the Python script at a temporary location.
  • Move the script to its pre-framework location.
  • Adapt the script content for 2.28. Maybe dependencies_of_setting needs more 2.28-only stuff but we can do that later based on looking at outcome analysis.
  • Check in the generated files, since we do that in 2.28.
  • Add the generated files to check-generated-files. In 2.28, there is no handling of generated files in **/Makefile and **/CMakeLists.txt, and no framework submodule to update.

PR checklist

Please tick as appropriate and edit the reasons (e.g.: "backport: not needed because this is a new feature")

  • [x] changelog no (test only)
  • [x] backport of https://github.com/Mbed-TLS/mbedtls/pull/9172
  • [x] tests provided

gilles-peskine-arm avatar Jun 26 '24 18:06 gilles-peskine-arm

I don't think it's worth it to enforce the same level of configuration testing in 2.28. And in that respect, I didn't go digging for options with dependencies in 2.28 (options where A requires B so we want to check A:B and !A:B, not just A and !A), not even to the small level I did for 3.6. But to even evaluate this, I need to get the reports from this pull request. Doing the backport was only about ½h of engineering time, plus the time to review it. I think that's well worth it.

gilles-peskine-arm avatar Jun 27 '24 12:06 gilles-peskine-arm

@CodiumAI-Agent /review

coolleng2525 avatar Jun 28 '24 09:06 coolleng2525

PR Reviewer Guide 🔍

⏱️ Estimated effort to review [1-5] 4
🧪 Relevant tests Yes
🔒 Security concerns No
⚡ Key issues to review Possible Bug:
The PR introduces a new script generate_config_tests.py which generates test cases for configuration settings. It is crucial to ensure that this script correctly handles all edge cases, especially since it deals with conditional dependencies and complex configurations.
Code Complexity:
The config.py script has been modified to include handling for inclusion guards and other complex parsing logic. This increases the complexity of the script, which could lead to maintenance challenges in the future.
Generated Data Verification:
The PR includes a large amount of automatically generated test data. It is important to verify that this data is correct and covers all necessary scenarios.

CodiumAI-Agent avatar Jun 28 '24 09:06 CodiumAI-Agent