Skip to content

Fix error checking in posix_spawn implementation of Command #77717

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
Merged
merged 2 commits into from
Oct 10, 2020
Merged

Fix error checking in posix_spawn implementation of Command #77717

merged 2 commits into from
Oct 10, 2020

Conversation

tmiasko
Copy link
Contributor

@tmiasko tmiasko commented Oct 8, 2020 •
edited
Loading

  • Check for errors returned from posix_spawn*_init functions
  • Check for non-zero return value from posix_spawn functions

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 8, 2020
cuviper
cuviper reviewed Oct 8, 2020
View reviewed changes
@tmiasko tmiasko force-pushed the posix-spawn-error-check branch from a25e367 to 3329831 Compare October 8, 2020 21:46
tmiasko added 2 commits October 8, 2020 23:53
@tmiasko
Check for non-zero return value from posix_spawn functions
5faf25b 
The cvt function compares the argument with -1 and when equal returns a new
io::Error constructed from errno. It is used together posix_spawn_* functions.
This is incorrect. Those functions do not set errno. Instead they return
non-zero error code directly.

Check for non-zero return code and use it to construct a new io::Error.
@tmiasko
Check for errors returned from posix_spawn*_init functions
6cd5506 
The posix_spawnattr_init & posix_spawn_file_actions_init might fail,
but their return code is not checked.

Check for non-zero return code and destroy only succesfully initialized
objects.
@tmiasko tmiasko force-pushed the posix-spawn-error-check branch from 3329831 to 6cd5506 Compare October 8, 2020 21:53
@cuviper
Copy link
Member

cuviper commented Oct 9, 2020

Looks nice, thanks!

@bors r+

@bors
Copy link
Collaborator

bors commented Oct 9, 2020

📌 Commit 6cd5506 has been approved by cuviper

@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-review Status: Awaiting review from the assignee but also interested parties. labels Oct 9, 2020
@bors
Copy link
Collaborator

bors commented Oct 10, 2020

⌛ Testing commit 6cd5506 with merge 7477d44...

@bors
Copy link
Collaborator

bors commented Oct 10, 2020

☀️ Test successful - checks-actions, checks-azure
Approved by: cuviper
Pushing 7477d44 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Oct 10, 2020
@bors bors merged commit 7477d44 into rust-lang:master Oct 10, 2020
@rustbot rustbot added this to the 1.49.0 milestone Oct 10, 2020
@tmiasko tmiasko deleted the posix-spawn-error-check branch October 10, 2020 13:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cuviper cuviper cuviper left review comments

Assignees

@cuviper cuviper

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.
Projects
None yet
Milestone
1.49.0
Development

Successfully merging this pull request may close these issues.
5 participants
@tmiasko @cuviper @bors @rust-highfive @rustbot