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

VMS knows POSIX threads too! #20440

Closed
wants to merge 2 commits into from
Closed

Conversation

levitte
Copy link
Member

@levitte levitte commented Mar 6, 2023

`include/internal/thread_arch.h didn't indicate this, now it does.

This also invalidates ossl_crypto_mem_barrier(), which we isn't used
anywhere. Reason for invalidation: it doesn't build with compilers
that don't support the GNU extension __asm__.

include/internal/thread_arch.h didn't indicate this, now it does.

This also invalidates ossl_crypto_mem_barrier(), which we isn't used
anywhere.  Reason for invalidation: it doesn't build with compilers
that don't support the GNU extension __asm__.
@levitte levitte added branch: master Merge to master branch approval: review pending This pull request needs review by a committer approval: otc review pending This pull request needs review by an OTC member labels Mar 6, 2023
@levitte levitte self-assigned this Mar 6, 2023
@levitte levitte requested review from a team March 6, 2023 08:11
@levitte levitte added the severity: urgent Fixes an urgent issue (exempt from 24h grace period) label Mar 6, 2023
@levitte
Copy link
Member Author

levitte commented Mar 6, 2023

I'm making this urgent, 'cause this is a very clear bug that make all testing fail

@github-actions github-actions bot added the severity: fips change The pull request changes FIPS provider sources label Mar 6, 2023
Copy link
Member

@beldmit beldmit left a comment

Choose a reason for hiding this comment

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

Why putting ossl_crypto_mem_barrier under #if 0 and not just removing?

@beldmit
Copy link
Member

beldmit commented Mar 6, 2023

The VMS part looks good to me

@levitte
Copy link
Member Author

levitte commented Mar 6, 2023

Why putting ossl_crypto_mem_barrier under #if 0 and not just removing?

Because I don't know the intent with it. Will we ever use it?

@levitte
Copy link
Member Author

levitte commented Mar 6, 2023

... but ok, I hear two voices saying that ossl_crypto_mem_barrier can simply be removed...

ossl_crypto_mem_barrier is now REMOVED, not just invalidated
@levitte
Copy link
Member Author

levitte commented Mar 6, 2023

The test_cmp_http failure on MacOS 11 / x86_64 isn't related to this PR.

Copy link
Member

@beldmit beldmit left a comment

Choose a reason for hiding this comment

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

LGTM

@levitte levitte removed the approval: otc review pending This pull request needs review by an OTC member label Mar 6, 2023
Copy link
Member

@mattcaswell mattcaswell left a comment

Choose a reason for hiding this comment

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

Ok with urgent.

@mattcaswell
Copy link
Member

@beldmit are you ok with urgent?

@beldmit
Copy link
Member

beldmit commented Mar 6, 2023

Yes, I agree with urgent

@levitte levitte removed the approval: review pending This pull request needs review by a committer label Mar 6, 2023
@levitte
Copy link
Member Author

levitte commented Mar 6, 2023

Side note: I see a trend in forgetting to deal with the labels. Can we please stop this trend?

@levitte levitte added the approval: ready to merge The 24 hour grace period has passed, ready to merge label Mar 6, 2023
@levitte
Copy link
Member Author

levitte commented Mar 6, 2023

Merged

ac21c17 VMS knows POSIX threads too!

@levitte levitte closed this Mar 6, 2023
@levitte levitte deleted the fix-VMS-threads branch March 6, 2023 11:21
openssl-machine pushed a commit that referenced this pull request Mar 6, 2023
include/internal/thread_arch.h didn't indicate this, now it does.

This also removes ossl_crypto_mem_barrier(), because we isn't used
anywhere, and doesn't build with compilers that don't support the GNU
extension __asm__.

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Dmitry Belyavskiy <beldmit@gmail.com>
(Merged from #20440)
@t8m t8m added the triaged: bug The issue/pr is/fixes a bug label Mar 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approval: ready to merge The 24 hour grace period has passed, ready to merge branch: master Merge to master branch severity: fips change The pull request changes FIPS provider sources severity: urgent Fixes an urgent issue (exempt from 24h grace period) triaged: bug The issue/pr is/fixes a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants