AIX doesn't provide fstatat64() - #1795
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dcb802013c
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| * that slip through capture remain in the private directory until | ||
| * cleanup — they are never lost or permanently removed. | ||
| */ | ||
| logg(LOGG_DEBUG, "traverse_unlink: No-replace rename unavailable on AIX; proceeding with linkat()-based restore for regular file '%s'.\n", target); |
There was a problem hiding this comment.
On AIX when quarantining/removing a validated regular file from a directory that another process can write to, this branch still leaves a race after the current_stat check and before the plain renameat(): the basename can be swapped to a directory, renameat() moves that directory into the private dir, and because supports_noreplace_restore is false, action_restore_captured_unlink_target() cannot restore it. The cleanup path only attempts to rmdir the private dir, so it fails once that captured directory is inside, leaving the replacement hidden and reintroducing the guarantee the non-AIX path preserves by bailing out.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
The _AIX bypass has been removed. Only the fstatat/stat64at fix remains in the PR.
dcb8020 to
322f97e
Compare
|
Can someone please take a look at this. |
val-ms
left a comment
val-ms
left a comment
There was a problem hiding this comment.
Please also verify that testing passes for AIX on your side since we don't have a system to test with.
| /* AIX: fstatat64 does not exist. Use stat64at which takes struct stat64* | ||
| * and is the 64-bit equivalent of fstatat. */ | ||
| rc = stat64at(dirfd, path, st, flags); | ||
| #elif defined(HAVE_STAT64) && STAT64_OK | ||
| rc = fstatat64(dirfd, path, st, flags); | ||
| #else | ||
| rc = fstatat(dirfd, path, st, flags); |
There was a problem hiding this comment.
I believe the stat64 guard is still needed.
Note: I also removed the comment in my proposed change, as it seems obvious.
| /* AIX: fstatat64 does not exist. Use stat64at which takes struct stat64* | |
| * and is the 64-bit equivalent of fstatat. */ | |
| rc = stat64at(dirfd, path, st, flags); | |
| #elif defined(HAVE_STAT64) && STAT64_OK | |
| rc = fstatat64(dirfd, path, st, flags); | |
| #else | |
| rc = fstatat(dirfd, path, st, flags); | |
| #if defined(HAVE_STAT64) && STAT64_OK && defined(_AIX) | |
| rc = stat64at(dirfd, path, st, flags); | |
| #elif defined(HAVE_STAT64) && STAT64_OK | |
| rc = fstatat64(dirfd, path, st, flags); | |
| #else | |
| rc = fstatat(dirfd, path, st, flags); |
2 participants
AIX exposes
stat64at()as the 64-bit directory-relative stat callinstead of
fstatat64(). Add adefined(_AIX)guard ahead of theexisting
HAVE_STAT64branch so thatstat64at()is called on AIX.