zos,fs: check F_OK separately in uv_fs_access()#3410
zos,fs: check F_OK separately in uv_fs_access()#3410zsw007 wants to merge 2 commits intolibuv:v1.xfrom
Conversation
On z/OS, F_OK is 8 instead of 0, so it is necessary to separate the access checks by creating an internal `uv__fs_access()` function. Co-Authored-By: Gaby Baghdadi <[email protected]>
This tests for the edge case where on certain platforms using F_OK to check that test_file exists will succeed, but checking that it also grants read and write permissions using F_OK | R_OK | W_OK will fail.
|
|
||
| #ifdef __MVS__ | ||
| static ssize_t uv__fs_access(uv_fs_t* req) { | ||
| /* On z/OS, F_OK is 8 instead of 0, so separate the access checks here. */ |
There was a problem hiding this comment.
It's not immediately clear (at least to me) just from reading this comment why F_OK being 8 requires special treatment. The docs I found say:
If you are using F_OK to test for the file's existence, you cannot use OR with any of the other symbols.
Maybe the comment could say "F_OK cannot be OR'ed with other access modes on z/OS."
There was a problem hiding this comment.
F_OK not being allowed to be OR'ed with other access modes does not appear to be a z/OS specific limitation.
e.g. https://linux.die.net/man/2/access
The mode specifies the accessibility check(s) to be performed, and is either the value F_OK, or a mask consisting of the bitwise OR of one or more of R_OK, W_OK, and X_OK.
The value of amode is either the bitwise-inclusive OR of the access permissions to be checked (R_OK, W_OK, X_OK) or the existence test (F_OK).
There was a problem hiding this comment.
I should have checked the docs for other platforms 🤦 ... in that case, do we even need to do anything here?
| access_cb_count = 0; /* reset for the next test */ | ||
|
|
||
| /* File should grant read and write permission */ | ||
| r = uv_fs_access(NULL, &req, "test_file", F_OK | R_OK | W_OK, NULL); |
There was a problem hiding this comment.
My reading of https://linux.die.net/man/2/access and https://pubs.opengroup.org/onlinepubs/9699919799/functions/access.html suggests that the F_OK | R_OK | W_OK combination of flags is not valid (not just on z/OS) and not expected to work. @zsw007 Is there some context as to the reason this PR was created -- perhaps z/OS was doing the right thing all along?
There was a problem hiding this comment.
In Node.js, under the parallel/test-fs-access tests, F_OK | R_OK | W_OK is being tested for on line 156:
This Node.js test case currently passes on platforms other than z/OS, and the changes in this PR is needed for the test case to pass on z/OS as well. The extra test case that I added to this PR is based off that Node.js test case (which currently also passes on platforms other than z/OS, and needs this PR to pass on z/OS).
But if you're saying that F_OK | R_OK | W_OK is not valid, then I'm wondering what's the reason for it being tested in Node.js?
There was a problem hiding this comment.
In Node.js, under the parallel/test-fs-access tests,
F_OK | R_OK | W_OKis being tested for on line 156:This Node.js test case currently passes on platforms other than z/OS, and the changes in this PR is needed for the test case to pass on z/OS as well. The extra test case that I added to this PR is based off that Node.js test case (which currently also passes on platforms other than z/OS, and needs this PR to pass on z/OS).
But if you're saying that
F_OK | R_OK | W_OKis not valid, then I'm wondering what's the reason for it being tested in Node.js?
The Node.js test appears to have been added in nodejs/node#114 which was authored by... @cjihrig 😄.
There was a problem hiding this comment.
Past Colin comes back to haunt me again. I'll open a PR to fix the Node.js test.
There was a problem hiding this comment.
Ah ok, so if ORing F_OK is a mistake in that test case, then it looks like this PR isn't necessary and the test case can be fixed instead 🙂
access() does not support OR'ing F_OK with other constants. This commit updates test-fs-access.js to not test that scenario. PR-URL: nodejs#41484 Refs: libuv/libuv#3410 Reviewed-By: Richard Lau Reviewed-By: Luigi Pinca Reviewed-By: Tobias Nießen
This commit expands the documentation for the mode parameter passed to the fs.access() family of functions. PR-URL: nodejs#41484 Refs: libuv/libuv#3410 Reviewed-By: Richard Lau Reviewed-By: Luigi Pinca Reviewed-By: Tobias Nießen
access() does not support OR'ing F_OK with other constants. This commit updates test-fs-access.js to not test that scenario. PR-URL: #41484 Refs: libuv/libuv#3410 Reviewed-By: Richard Lau Reviewed-By: Luigi Pinca Reviewed-By: Tobias Nießen
This commit expands the documentation for the mode parameter passed to the fs.access() family of functions. PR-URL: #41484 Refs: libuv/libuv#3410 Reviewed-By: Richard Lau Reviewed-By: Luigi Pinca Reviewed-By: Tobias Nießen
access() does not support OR'ing F_OK with other constants. This commit updates test-fs-access.js to not test that scenario. PR-URL: nodejs#41484 Refs: libuv/libuv#3410 Reviewed-By: Richard Lau Reviewed-By: Luigi Pinca Reviewed-By: Tobias Nießen
This commit expands the documentation for the mode parameter passed to the fs.access() family of functions. PR-URL: nodejs#41484 Refs: libuv/libuv#3410 Reviewed-By: Richard Lau Reviewed-By: Luigi Pinca Reviewed-By: Tobias Nießen
access() does not support OR'ing F_OK with other constants. This commit updates test-fs-access.js to not test that scenario. PR-URL: nodejs#41484 Refs: libuv/libuv#3410 Reviewed-By: Richard Lau Reviewed-By: Luigi Pinca Reviewed-By: Tobias Nießen
This commit expands the documentation for the mode parameter passed to the fs.access() family of functions. PR-URL: nodejs#41484 Refs: libuv/libuv#3410 Reviewed-By: Richard Lau Reviewed-By: Luigi Pinca Reviewed-By: Tobias Nießen
access() does not support OR'ing F_OK with other constants. This commit updates test-fs-access.js to not test that scenario. PR-URL: nodejs#41484 Refs: libuv/libuv#3410 Reviewed-By: Richard Lau Reviewed-By: Luigi Pinca Reviewed-By: Tobias Nießen
This commit expands the documentation for the mode parameter passed to the fs.access() family of functions. PR-URL: nodejs#41484 Refs: libuv/libuv#3410 Reviewed-By: Richard Lau Reviewed-By: Luigi Pinca Reviewed-By: Tobias Nießen
access() does not support OR'ing F_OK with other constants. This commit updates test-fs-access.js to not test that scenario. PR-URL: #41484 Refs: libuv/libuv#3410 Reviewed-By: Richard Lau Reviewed-By: Luigi Pinca Reviewed-By: Tobias Nießen
This commit expands the documentation for the mode parameter passed to the fs.access() family of functions. PR-URL: #41484 Refs: libuv/libuv#3410 Reviewed-By: Richard Lau Reviewed-By: Luigi Pinca Reviewed-By: Tobias Nießen
3 participants
On z/OS,
F_OKis 8 instead of 0. This PR creates an internaluv__fs_access()function, where the access check forF_OKneeds to be separated fromR_OK,W_OK, andX_OKon z/OS. Otherwise, checking that a file grants read and write permissions usingF_OK | R_OK | W_OKwill fail.Also updates the
fs_accesstest to include this edge case.