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

2.28 only: build broken when check_config.h is not included #9152

Closed
gilles-peskine-arm opened this issue May 17, 2024 · 1 comment
Closed
Assignees
Labels
bug component-platform Portability layer and build scripts priority-high High priority - will be reviewed soon size-xs Estimated task size: extra small (a few hours at most)

Comments

@gilles-peskine-arm
Copy link
Contributor

gilles-peskine-arm commented May 17, 2024

In Mbed TLS 2.x, including mbedtls/check_config.h is optional. We do it in the default configuration file, but users can provide their own configuration file that doesn't include it.

The general philosophy of check_config.h is to have no side effects, but that's not completely true:

  • On Windows (#if defined(_WIN32)), it might define MBEDTLS_PLATFORM_SNPRINTF_ALT and MBEDTLS_PLATFORM_VSNPRINTF_ALT.
  • It includes limits.h.

We need to ensure that these side effects don't matter. They do as of Mbed TLS 2.28.7: at least oid.c uses UINT_MAX but does not include limits.h, which breaks the build when check_config.h is not included. See openthread/openthread#10263 (comment)

This is not an issue in Mbed TLS ≥3.0 because there check_config.h has no side effects: the side effects happen in build_info.h.

Almost all of our CI builds (if not all?) include check_config.h. So we might not notice if a build breaks when check_config.h is not included. The goal of this issue is to:

  • Do a build with check_config.h omitted in all.sh.
  • Fix oid.c and any other problem.
  • What about the Windows (v)snprintf alt stuff?
@gilles-peskine-arm gilles-peskine-arm added bug component-platform Portability layer and build scripts priority-high High priority - will be reviewed soon size-xs Estimated task size: extra small (a few hours at most) labels May 17, 2024
@gilles-peskine-arm gilles-peskine-arm self-assigned this May 17, 2024
@gilles-peskine-arm
Copy link
Contributor Author

Resolved by #9153

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug component-platform Portability layer and build scripts priority-high High priority - will be reviewed soon size-xs Estimated task size: extra small (a few hours at most)
Projects
Status: 3.6.1 patch release
Development

No branches or pull requests

1 participant