[PATCH] D64236: [llvm-objcopy] Don't change permissions of non-regular output files
Fangrui Song via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Jul 10 07:35:46 PDT 2019
MaskRay added a comment.
In D64236#1578315 <https://reviews.llvm.org/D64236#1578315>, @jhenderson wrote:
> > I just built this and ran the tests on my Windows machine, and saw no issues. However, I'm not sure I follow why you'd expect there to be any issues with the current state of the code?
> >
> > I'm happy with the Windows side not being implemented for now, but I'm also not sure why you need the new overload at all? Isn't the OStat.type() check sufficient?
>
> Thanks for the update. Can you answer these comments, please?
With the current state of the code, `objcopy a.o /dev/null` => EPERM because it tries to set the mode bits of /dev/null (`/dev/null` is owned by root).
If you are root, you can even change its mode bits (usually `rw-rw-rw-`).
The new overload saves one `stat` syscall and avoids the race condition (if the filename is moved between `sys::fs::openFileForWrite` and `sys::fs::setPermissions`, the updated code will not operate on a nonexistent/different file). I'm sorry that I didn't read carefully when `restoreStatOnFile` was initially checked in.
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D64236/new/
https://reviews.llvm.org/D64236
More information about the llvm-commits
mailing list