Implement OpenSSL secure memory for Windows #13172
Conversation
e_os.h
Outdated
| && ( (defined(_POSIX_VERSION) && _POSIX_VERSION >= 200112L) \ | ||
| || defined(__sun) || defined(__hpux) || defined(__sgi) \ | ||
| || defined(__osf__) ) | ||
| || defined(__osf__) )) || defined(_WIN32) |
There was a problem hiding this comment.
Break the || defined(_WIN32) onto the next line. The indentation is also off with the addition of the initial (.
There was a problem hiding this comment.
I'm hoping I got this right now, can you double check me please?
|
Great, happy to fix up those nits! Any other feedback is welcome. |
|
If you don't mind, could you possibly label this PR as 'hacktoberfest-accepted'? I did not make this pull request for the purpose of hacktoberfest, but I am participating. Thanks! |
Could you explain what the label actually means? I'm not familiar with it. |
|
And how to add it... |
|
It's just a label on the PR that allows the PR to count towards the goals of https://hacktoberfest.digitalocean.com which is a challenge for people to contribute towards open source projects in October. It's not a big deal, but if you would set a label field on the PR to that string I would get credit for it. The itself PR is what matters though. I didn't make the PR for the sake of Hacktoberfest. |
|
I am not sure we want to add such label to our already quite long list of labels. They suggest that making a PR on your own repository fork will be also counted. |
|
According to https://hacktoberfest.digitalocean.com
it needs to be a GitHub topic not a label. |
|
C'mon @openssl/omc, why not add it? It might attract lotsofpullrequests ;-) |
|
@openssl/omc talking about topics: maybe we should also consider adding also other more serious topics to our repo? |
Ok, so there is also a label for explicit opt-in: https://hacktoberfest.digitalocean.com/hacktoberfest-update |
mattcaswell
added
approval: review pending
|
@mattcaswell @paulidale I think this deserves a hacktoberfest-accepted label when it gets the necessary approvals, what do you think? |
|
I'm ok with that. |
|
I'm fine with the label for this or for the project to participate (although I worry about attracting lots of extra work right now). |
|
Don’t worry @paulidale, only ten days to go ;-) |
paulidale
added
approval: done
|
I've posited the hacktoberfest question to the project list. |
@jgowdy thank you for your contribution! Your pull request has reached the required number of approvals and will be merged after the 24 hour grace period. @mattcaswell and @paulidale already indicated consent to your request for a Hacktoberfest badge, but I'll wait another day or so to add it, in order not to preempt the discussion on the mailing list. |
|
Excellent, thank you all! |
|
I've actually completed the Hacktoberfest challenge already, so we can skip the label. I don't want to pollute the label list for no reason. I appreciate the consideration. |
Ok, thanks to you for pointing us to the Hacktoberfest challenge. At the least, we will take a look and consider whether it makes sense to add the Hacktoberfest topic to the repo next year. |
|
24 hours has passed since 'approval: done' was set, but as this PR has been updated in that time the label 'approval: ready to merge' is not being automatically set. Please review the updates and set the label manually. |
Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Paul Dale <paul.dale@oracle.com> (Merged from #13172)
paulidale
added
approval: ready to merge
|
Merged to master. Thanks. |
Hello, please review this pull request implementing CRYPTO_secure_malloc and the associated functions for the Windows platform using equivalent functions such as VirtualAlloc for mmap, etc. Thank you.
Checklist
Documentation for CRYPTO_secure_malloc(3) doesn't seem to mention which platforms are and are not supported, so I do not believe any update is required.
Existing secmemtest should cover Windows implementation