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

Check time platform abstraction macro definitions #534

Merged
merged 4 commits into from Jul 20, 2016

Conversation

andresag01
Copy link
Contributor

This patch adds some checks to check_config.h to ensure that macro
definitions for the time platform abstraction are acceptable. In this
case the requirements are:

  • MBEDTLS_PLATFORM_C must be defined whenever
    MBEDTLS_PLATFORM_TIME_ALT, MBEDTLS_PLATFORM_TIME_TYPE_MACRO or
    MBEDTLS_PLATFORM_TIME_MACRO is defined.
  • MBEDTLS_PLATFORM_STD_TIME and MBEDTLS_PLATFORM_TIME_ALT cannot be
    defined simultaneously with MBEDTLS_PLATFORM_TIME_TYPE_MACRO or
    MBEDTLS_PLATFORM_TIME_MACRO.

Andres AG added 2 commits July 14, 2016 14:37
This patch adds some checks to check_config.h to ensure that macro
definitions for the time platform abstraction are acceptable. In this
case the requirements are:
  - MBEDTLS_PLATFORM_C and MBEDTLS_HAVE_TIME must be defined whenever
    MBEDTLS_PLATFORM_TIME_ALT, MBEDTLS_PLATFORM_TIME_TYPE_MACRO or
    MBEDTLS_PLATFORM_TIME_MACRO is defined.
  - MBEDTLS_PLATFORM_STD_TIME and MBEDTLS_PLATFORM_TIME_ALT cannot be
    defined simultaneously with MBEDTLS_PLATFORM_TIME_TYPE_MACRO or
    MBEDTLS_PLATFORM_TIME_MACRO.
  - MBEDTLS_HAVE_TIME and MBEDTLS_PLATFORM_TIME_ALT must be defined
    whenever MBEDTLS_PLATFORM_STD_TIME is defined.
Document that time platform abstraction macros
MBEDTLS_PLATFORM_TIME_ALT, MBEDTLS_PLATFORM_TIME_MACRO,
MBEDTLS_PLATFORM_TIME_TYPE_MACRO and MBEDTLS_PLATFORM_STD_TIME require
MBEDTLS_HAVE_TIME to be defined in config.h.
@andresag01
Copy link
Contributor Author

@yanesca @sbutcher-arm

@yanesca
Copy link
Contributor

yanesca commented Jul 15, 2016

Why is MBEDTLS_PLATFORM_C a prerequisite for MBEDTLS_PLATFORM_TIME_TYPE_MACRO?

@yanesca
Copy link
Contributor

yanesca commented Jul 15, 2016

Why is the following condition necessary?
"MBEDTLS_HAVE_TIME and MBEDTLS_PLATFORM_TIME_ALT must be defined
whenever MBEDTLS_PLATFORM_STD_TIME is defined."

( defined(MBEDTLS_PLATFORM_STD_TIME) ||\
defined(MBEDTLS_PLATFORM_TIME_ALT) )
#error "MBEDTLS_PLATFORM_TIME_MACRO/MBEDTLS_PLATFORM_TIME_TYPE_MACRO and MBEDTLS_PLATFORM_STD_TIME/MBEDTLS_PLATFORM_TIME_ALT cannot be defined simultaneously"
#endif
Copy link
Contributor

Choose a reason for hiding this comment

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

I would prefer to have this condition separated, like the other one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree that this check is more complex than the other ones because we need to check both MBEDTLS_PLATFORM_TIME_MACRO and MBEDTLS_PLATFORM_TIME_TYPE_MACRO. I put them both in th same check because they are really related.

@simonbutcher
Copy link
Contributor

+1

1 similar comment
@yanesca
Copy link
Contributor

yanesca commented Jul 19, 2016

+1

@andresag01
Copy link
Contributor Author

The last commit splits the rather complex condition in check_config.h into two separate preprocessor conditions for simplicity as suggested by @yanesca.

@simonbutcher simonbutcher added the approved Design and code approved - may be waiting for CI or backports label Jul 20, 2016
@simonbutcher
Copy link
Contributor

Reviewed and approved for merge,

@simonbutcher simonbutcher merged commit 1e4ec66 into Mbed-TLS:development Jul 20, 2016
simonbutcher added a commit that referenced this pull request Nov 20, 2018
iameli pushed a commit to livepeer/mbedtls that referenced this pull request Dec 5, 2023
…6.04

ci: fix meson build on ubuntu 16.04
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 bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants