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

[Alex Brachet via Phabricator via llvm-commits]

abrachet added a comment.

>   but I'm also not sure why you need the new overload at all? Isn't the OStat.type() check sufficient?

I agree with @jhenderson here. If we don't implement a `setPermissions(int, perms)` for Windows, I reckon we should only use `setPermissions(const Twine&, perms)`.



================
Comment at: lib/Support/Windows/Path.inc:773
+std::error_code setPermissions(int FD, perms Permissions) {
+  // FIXME Not implemented.
+  return std::error_code();
----------------
jhenderson wrote:
> MaskRay wrote:
> > abrachet wrote:
> > > By no means a Windows expert, but it seems like [[ https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/get-osfhandle?view=vs-2019 | _get_osfhandle ]] and [[ https://docs.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-setfileinformationbyhandle | SetFileInformationByHandle ]] would be a place to start?
> > I don't even have a Windows for testing... I'll leave this to a Windows expert.
> Perhaps it's worth return a not_supported error here?
We could do something hacky like this to get it to at least //work// on Windows.
```
HANDLE Handle = _get_osfhandle(FD);
char Buf[256];
GetFinalPathNameByHandleA(Handle, Buf, 256, 0);
return setPermissions(Buf, Permissions);
```


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64236/new/

https://reviews.llvm.org/D64236