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.

Sign up for GitHub

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

Jump to bottom

fix error handling for pthread_sigmask(3) #93530

Merged
merged 2 commits into from
Apr 18, 2022
Merged

fix error handling for pthread_sigmask(3) #93530

merged 2 commits into from
Apr 18, 2022

Conversation

anonion0
Copy link
Contributor

@anonion0 anonion0 commented Feb 1, 2022

Errors from pthread_sigmask(3) were handled using cvt(), which expects a return value of -1 on error and uses errno.
However, pthread_sigmask(3) returns 0 on success and an error number otherwise.

Fix it by replacing cvt() with cvt_nz().

@anonion0
fix error handling for pthread_sigmask(3)
c492355 
Errors from pthread_sigmask(3) were handled using cvt(), which expects a
return value of -1 on error and uses errno.
However, pthread_sigmask(3) returns 0 on success and an error number
otherwise.
Fix it by replacing cvt() with cvt_nz().
@rust-highfive
Copy link
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @kennytm (or someone else) soon.

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 1, 2022
@JohnCSimon JohnCSimon added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 6, 2022
@JohnCSimon JohnCSimon added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 11, 2022
@JohnTitor
Copy link
Member

Good catch, thanks!
r? @JohnTitor @bors r+

@bors
Copy link
Contributor

bors commented Apr 15, 2022

📌 Commit c492355 has been approved by JohnTitor

@rust-highfive rust-highfive assigned JohnTitor and unassigned kennytm Apr 15, 2022
@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 15, 2022
@bors
Copy link
Contributor

bors commented Apr 15, 2022

💡 This pull request was already approved, no need to approve it again.

@bors
Copy link
Contributor

bors commented Apr 15, 2022

📌 Commit c492355 has been approved by kennytm

RalfJung added a commit to RalfJung/rust that referenced this pull request Apr 15, 2022
@RalfJung
Rollup merge of rust-lang#93530 - anonion0:pthread_sigmask_fix, r=ken…
397c134 
…nytm

fix error handling for pthread_sigmask(3)

Errors from `pthread_sigmask(3)` were handled using `cvt()`, which expects a return value of `-1` on error and uses `errno`.
However, `pthread_sigmask(3)` returns `0` on success and an error number otherwise.

Fix it by replacing `cvt()` with `cvt_nz()`.
@RalfJung RalfJung mentioned this pull request Apr 15, 2022
Closed
@RalfJung
Copy link
Member

Failed in #96075 (comment)
@bors r- rollup=iffy

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Apr 15, 2022
@JohnTitor
Copy link
Member

@anonion0 Could you move these imports under cfgs?

@rustbot rustbot added the T-libs Relevant to the library team, which will review and decide on the PR/issue. label Apr 17, 2022
@anonion0
Copy link
Contributor Author

Should be fixed now.

@JohnTitor
Copy link
Member

Thanks!
@bors r+

@bors
Copy link
Contributor

bors commented Apr 17, 2022

📌 Commit e6aafbc has been approved by JohnTitor

@bors bors removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Apr 17, 2022
@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Apr 17, 2022
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Apr 17, 2022
@Dylan-DPC
Rollup merge of rust-lang#93530 - anonion0:pthread_sigmask_fix, r=Joh…
aeda51c 
…nTitor

fix error handling for pthread_sigmask(3)

Errors from `pthread_sigmask(3)` were handled using `cvt()`, which expects a return value of `-1` on error and uses `errno`.
However, `pthread_sigmask(3)` returns `0` on success and an error number otherwise.

Fix it by replacing `cvt()` with `cvt_nz()`.
@Dylan-DPC Dylan-DPC mentioned this pull request Apr 17, 2022
Closed
@bors
Copy link
Contributor

bors commented Apr 17, 2022

⌛ Testing commit e6aafbc with merge e27d9df...

@bors
Copy link
Contributor

bors commented Apr 18, 2022

☀️ Test successful - checks-actions
Approved by: JohnTitor
Pushing e27d9df to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Apr 18, 2022
@bors bors merged commit e27d9df into rust-lang:master Apr 18, 2022
@rustbot rustbot added this to the 1.62.0 milestone Apr 18, 2022
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (e27d9df): comparison url.

Summary:

  • Primary benchmarks: 🎉 relevant improvement found
  • Secondary benchmarks: no relevant changes found
Regressions 😿
(primary)		 Regressions 😿
(secondary)		 Improvements 🎉
(primary)		 Improvements 🎉
(secondary)		 All 😿 🎉
(primary)
count1 0 0 1 0 1
mean2 N/A N/A -3.0% N/A -3.0%
max N/A N/A -3.0% N/A -3.0%

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

@rustbot label: -perf-regression

Footnotes

  1. number of relevant changes ↩

  2. the arithmetic mean of the percent change ↩

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JohnTitor JohnTitor JohnTitor approved these changes

Assignees

@JohnTitor JohnTitor

Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.62.0
Development

Successfully merging this pull request may close these issues.
9 participants
@anonion0 @rust-highfive @JohnTitor @bors @RalfJung @rust-timer @kennytm @JohnCSimon @rustbot