pipe: fixed uv_pipe_chmod for OSX Darwin #1635
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
cjihrig
reviewed
Nov 20, 2017
src/unix/pipe.c
Outdated
| return r; | ||
| } | ||
|
|
||
| /* Unfortunately fstat on a pipe has a bug on OSX Darwin */ |
Contributor
There was a problem hiding this comment.
Nit: I don't think this comment provides enough information outside of the context of this PR. Perhaps at least say that stat is used instead of fstat due to a bug on macOS."
According to the fstat man page there is a bug in fstat on OS X that fstat on a pipe. The fstat() call had to be changed to a stat() call in order to get the correct permissions. Test cases have been modified to check the result of uv_pipe_chmod to confirm the change actually worked.
zzzjim
force-pushed
the
fix-pipe-chmod
branch
from
e43f0b5 to
361b518
Compare
Contributor
Author
|
I've fixed the comments for osx about fstat and stat. |
santigimeno
approved these changes
Apr 11, 2018
zzzjim
force-pushed
the
fix-pipe-chmod
branch
2 times, most recently
from
01d8d69 to
f8268f0
Compare
According to the fstat man page there is a bug in fstat on OS X that fstat on a pipe. The fstat() call had to be changed to a stat() call in order to get the correct permissions. Test cases have been modified to check the result of uv_pipe_chmod to confirm the change actually worked.
Contributor
Author
|
@cjihrig - rebased. |
cjihrig
pushed a commit
to cjihrig/libuv
that referenced
this pull request
Apr 12, 2018
According to its man page, there is a bug in fstat() on macOS related to pipes. This commit replaces a fstat() call in uv_pipe_chmod() with a stat() call in order to get the correct permissions. PR-URL: libuv#1635 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Santiago Gimeno <[email protected]>
Contributor
|
Landed in 19855c0. Thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
According to the fstat man page there is a bug
in fstat on OS X that fstat on a pipe.
The fstat() call had to be changed to a stat() call
in order to get the correct permissions.
Test cases have been modified to check the result
of uv_pipe_chmod to confirm the change actually worked.