-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Adds MBEDTLS_THREADING_C implementation for Windows using srwlock #7180
base: development
Are you sure you want to change the base?
Conversation
Signed-off-by: Frederik Wedel-Heinen <frederik.wedel-heinen@dencrypt.dk>
…OCK by default Signed-off-by: Frederik Wedel-Heinen <frederik.wedel-heinen@dencrypt.dk>
Signed-off-by: Frederik Wedel-Heinen <frederik.wedel-heinen@dencrypt.dk>
Signed-off-by: Frederik Wedel-Heinen <frederik.wedel-heinen@dencrypt.dk>
There are many test failures. Please check the logs on Travis and "TF OpenCI: PR-7180-head" (the "Internal CI" jobs are duplicates of the OpenCI ones because we're in a transition, don't worry about those). |
@gilles-peskine-arm |
…OCK for windows test build. And adds MBEDTLS_THREADING_SRWLOCK to the list of items that are excluded from a 'full config'. Signed-off-by: Frederik Wedel-Heinen <frederik.wedel-heinen@dencrypt.dk>
@fwh-dc Most of the testing is done by running |
Looked in to the remaining failures and seems to be:
WIN32_LEAN_AND_MEAN is only to make compilation faster so it could be removed. Should I make an opaque type Please let me know what you prefer. I'll fix the code style to fix the last bulletpoint. |
Signed-off-by: Frederik Wedel-Heinen <frederik.wedel-heinen@dencrypt.dk>
include/mbedtls/threading.h
Outdated
/* | ||
* Use spaces to pass the macroses name check. | ||
*/ | ||
# define WIN32_LEAN_AND_MEAN |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add WIN32_LEAN_AND_MEAN
and _WIN32_WINNT
to the exclusions
list in parse_macros
in tests/scripts/check_names.py
. That's how we deal with the similar case of _CRT_SECURE_NO_DEPRECATE
.
include/mbedtls/threading.h
Outdated
/* | ||
* Use spaces to pass the macroses name check. | ||
*/ | ||
# define WIN32_LEAN_AND_MEAN |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add a comment explaining why WIN32_LEAN_AND_MEAN
is good to have. After 30 seconds on the web, I guess it's to avoid conflicts with some other headers?
(For _WIN32_WINNT
, even without knowing anything about Windows programming, it's obvious to me that it specifies the API version that we're requesting, similar to #define _POSIX_C_SOURCE
on Unix-like systems.)
That would require all See my other comment for how to deal with |
…threading.h to enable srwlock api. Signed-off-by: Frederik Wedel-Heinen <frederik.wedel-heinen@dencrypt.dk>
Signed-off-by: Frederik Wedel-Heinen <frederik.wedel-heinen@dencrypt.dk>
b0282bf
to
c53a695
Compare
Signed-off-by: Frederik Wedel-Heinen <frederik.wedel-heinen@dencrypt.dk>
@gilles-peskine-arm Other than that, I'd like you to have a look at this and let me know if there's something more that needs fixing. |
There was a temporary problem with the license server. I've restarted the job. At first glance this seems to be in the right shape, but we'll need to do a full review. Please add a changelog entry (see We're only testing that the build works with What testing have you done locally? With what compiler(s) version(s)? |
Description
Adds default support for threading layer on Windows using srwlocks.
This work is based on a previous pull request #2306 and is rebased to the current tip of development and modified accordingly.
Gatekeeper checklist
Notes for the submitter
Please refer to the contributing guidelines, especially the
checklist for PR contributors.