mbedtls icon indicating copy to clipboard operation
mbedtls copied to clipboard

Include platform.h unconditionally

Open gilles-peskine-arm opened this issue 2 years ago • 0 comments

Include mbedtls/platform.h even when MBEDTLS_PLATFORM_C is disabled. See the discussion in #6199.

The systematic replacement I've made may cause mbedtls/platform.h to be included in configurations where it isn't needed at all. That's a lot simpler than digging through files and checking every case one by one. A spurious header inclusion doesn't really matter anyway.

With respect to the goals of #6199:

1. Document when mbedts/platform.h should be included.

The rule is now to include it whenever you use something it defines. This is the general rule so it doesn't really require any documentation. Nonetheless I've added a bit at the top of platform.h.

2. Ensure that we follow the rule.

I've removed the preprocessor conditionals around inclusions of mbedtls/platform.h, and the manual definitions of mbedtls_xxx alias macros in individual .c files.

3. If needed, add configurations to the CI, or tweak existing ones, to ensure we keep following the rule in the future.

The rule is now the obvious general rule for including headers so there's no particular reason why we'd get it wrong. Thus I haven't added any configuration on the CI.

This fixes the build in the configuration of https://github.com/Mbed-TLS/mbedtls/issues/6118 (to reproduce: make CFLAGS='-DMBEDTLS_CONFIG_FILE="6118.mbedtls_config.h"'). I've also fixed an unrelated build failure in this configuration, in test_suite_pkcs12. I haven't added a configuration on the CI for that because we can't test every combination and I don't think that one warrants yet another build on the CI, but I could be convinced otherwise.

  • Changelog: done.
  • Backport: TODO. As discussed in the issue I'm planning to go for a straightforward backport, i.e. we'll start including platform.h unconditionally in 2.28 as well.
  • Priority: high because it's a regression in the previous release.

gilles-peskine-arm avatar Sep 15 '22 19:09 gilles-peskine-arm