Skip to content

check_config.h: move to library and test #10319

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

Conversation

gilles-peskine-arm
Copy link
Contributor

@gilles-peskine-arm gilles-peskine-arm commented Jul 22, 2025

Needs preceding PR: Mbed-TLS/mbedtls-framework#192. The TF-PSA-Crypto and mbedtls PR can be merged independently.

PR checklist

@gilles-peskine-arm gilles-peskine-arm added needs-ci Needs to pass CI tests needs-preceding-pr Requires another PR to be merged first labels Jul 22, 2025
@gilles-peskine-arm gilles-peskine-arm added size-s Estimated task size: small (~2d) priority-high High priority - will be reviewed soon labels Jul 22, 2025
@gilles-peskine-arm gilles-peskine-arm 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 and removed needs-ci Needs to pass CI tests labels Jul 23, 2025
@valeriosetti valeriosetti self-requested a review July 23, 2025 07:35
@valeriosetti
Copy link
Contributor

I think that

Rename check_config.h to tf_psa_crypto_check_config.h

in the PR's description should be updated to mbedtls_check_config.h, right?

valeriosetti
valeriosetti previously approved these changes Jul 23, 2025
@gilles-peskine-arm
Copy link
Contributor Author

I've fixed the PR description. Let me know if you find similar copypasta in the code or in commit messages.

bjwtaylor
bjwtaylor previously approved these changes Jul 23, 2025
@github-project-automation github-project-automation bot moved this from In Development to Has Approval in Roadmap pull requests (new board) Jul 23, 2025
This will be populated in subsequent commits.

Signed-off-by: Gilles Peskine <[email protected]>
`check_config.h` only needs to run once on the configuration. It doesn't
need to run every time an application is built. It used to be public up to
Mbed TLS 2.x because it was included from `config.h`, and users could
substitute that file completely and should still include `check_config.h`
from their file. But since Mbed TLS 3.x, including `check_config.h` is a
purely internal thing (done in `build_info.h`). So make the file itself
purely internal.

We don't need to include `check_config.h` when building every library file,
just one: `mbedtls_config.c`, that's its job.

Give the file a unique name, to avoid any clashes with TF-PSA-Crypto's
`check_config.h`.

Signed-off-by: Gilles Peskine <[email protected]>
This will be needed when TF-PSA-Crypto's `build_info.h` stops including
`limits.h`, which it currently does by accident because it includes
`check_config.h` which wants `limits.h` to check `CHAR_BIT`.

Signed-off-by: Gilles Peskine <[email protected]>
Ensure that `mbedtls_check_config.h` is taken into account.

Signed-off-by: Gilles Peskine <[email protected]>
It was already deprecated since 3.0 (although we forgot to announce it in
the changelog back then).

Signed-off-by: Gilles Peskine <[email protected]>
@gilles-peskine-arm gilles-peskine-arm force-pushed the move-check-config-to-library branch from 256000d to fff4b32 Compare July 28, 2025 13:47
@gilles-peskine-arm
Copy link
Contributor Author

I rebased to handle the framework conflict.

@gilles-peskine-arm gilles-peskine-arm removed the needs-reviewer This PR needs someone to pick it up for review label Jul 29, 2025
@gilles-peskine-arm gilles-peskine-arm added approved Design and code approved - may be waiting for CI or backports and removed needs-review Every commit must be reviewed by at least two team members, needs-preceding-pr Requires another PR to be merged first labels Jul 29, 2025
@gilles-peskine-arm gilles-peskine-arm added this pull request to the merge queue Jul 29, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jul 29, 2025
@gilles-peskine-arm
Copy link
Contributor Author

The merge queue failed in what looks like a new variant of a DTLS intermittent failure. I'll requeue.

@gilles-peskine-arm gilles-peskine-arm added this pull request to the merge queue Jul 29, 2025
Merged via the queue into Mbed-TLS:development with commit d6f881e Jul 29, 2025
4 of 6 checks passed
@github-project-automation github-project-automation bot moved this from Has Approval to Done in Roadmap pull requests (new board) Jul 29, 2025
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 priority-high High priority - will be reviewed soon size-s Estimated task size: small (~2d)
Development

Successfully merging this pull request may close these issues.

Move the inclusion of check_config.h to library
3 participants