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

config: Allow Mbed to implement TIMING_C #4634

Merged
merged 1 commit into from
Jun 11, 2021

Conversation

Patater
Copy link
Contributor

@Patater Patater commented Jun 9, 2021

Description

Mbed OS now provides POSIX-like time functions, although not alarm() nor
signal(). It is possible to implement MBEDTLS_TIMING_ALT on Mbed OS, so
we should not artificially prevent this in check-config. Remove the the
check that prevents implementing MBEDTLS_TIMING_ALT on Mbed OS.

Note that this limitation originally was added in the following commit,
although there isn't much context around why the restriction was
imposed: 63e7eba ("Add material for generating yotta module"). In
2015, Mbed OS was quite a different thing: no RTOS, no threads, just an
asynchronous event loop model. I'd suppose the asynchronous event loop
model made it difficult before to implement MBEDTLS_TIMING_C on Mbed OS,
but that is no longer the case.

Fixes #4633

Signed-off-by: Jaeden Amero [email protected]

Status

READY

Requires Backporting

When there is a bug fix, it should be backported to all maintained and supported branches.
Changes do not have to be backported if:

  • This PR is a new feature\enhancement
  • This PR contains changes in the API. If this is true, and there is a need for the fix to be backported, the fix should be handled differently in the legacy branch

Yes

  • development_2.x

Others branches if desired, although likely not useful because Mbed OS doesn't integrate versions other than the latest 2.x at the moment and this bug fix is only relevant for Mbed OS.

Migrations

NO

Additional comments

Any additional information that could be of interest

Todos

  • [ ] Tests Removing unnecessary check-config check
  • [ ] Documentation
  • Changelog updated
  • Backported

Steps to test or reproduce

Implement MBEDTLS_TIMING_ALT for Mbed OS, and get blocked by this legacy check which doesn't make sense anymore.

Mbed OS now provides POSIX-like time functions, although not alarm() nor
signal(). It is possible to implement MBEDTLS_TIMING_ALT on Mbed OS, so
we should not artificially prevent this in check-config. Remove the the
check that prevents implementing MBEDTLS_TIMING_ALT on Mbed OS.

Note that this limitation originally was added in the following commit,
although there isn't much context around why the restriction was
imposed: 63e7eba ("Add material for generating yotta module"). In
2015, Mbed OS was quite a different thing: no RTOS, no threads, just an
asynchronous event loop model. I'd suppose the asynchronous event loop
model made it difficult before to implement MBEDTLS_TIMING_C on Mbed OS,
but that is no longer the case.

Fixes Mbed-TLS#4633

Signed-off-by: Jaeden Amero <[email protected]>
@gilles-peskine-arm gilles-peskine-arm added bug component-platform Portability layer and build scripts needs-backports Backports are missing or are pending review and approval. 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 labels Jun 9, 2021
@Patater
Copy link
Contributor Author

Patater commented Jun 9, 2021

Thanks for the quick review. Backport raised to development_2.x at #4635

Copy link
Contributor

@stevew817 stevew817 left a comment

Choose a reason for hiding this comment

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

Am I the only one who finds it strange to see an mbed-specific check in an otherwise target-agnostic file? Shouldn't this rather be checked by the adaptation layer calling mbed TLS from mbed OS?

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 needs-backports Backports are missing or are pending review and approval. 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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

check-config.h blocks implementation of timing on Mbed OS
4 participants