[PATCH] D64236: [llvm-objcopy] Don't change permissions of non-regular output files

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