[PATCH] D98426: [llvm-objcopy][Support] move writeToStream helper function to Support.

Haojian Wu via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jun 23 10:48:43 PDT 2023


hokein added inline comments.


================
Comment at: llvm/lib/Support/raw_ostream.cpp:1003
+
+  unsigned Mode = sys::fs::all_read | sys::fs::all_write | sys::fs::all_exe;
+  Expected<sys::fs::TempFile> Temp =
----------------
avl wrote:
> hokein wrote:
> > is the all_exe mode intended here? (the clang-include-cleaner tool is using this API, I was debugging an issue where the file type of the modified file was changed executable).
> yep, It looks incorrectly to set it by default. It would be better to remove it or make configurable.
> 
> there is similar API llvm::writeFileAtomically. probably it would be better to unite both of this APIs.
> yep, It looks incorrectly to set it by default. It would be better to remove it or make configurable.

thanks. I will sent out a patch. 

> there is similar API llvm::writeFileAtomically. probably it would be better to unite both of this APIs.

Sounds good to me. There is only 2 in-tree usages of `llvm::writeFileAtomically` API, I think we can even just remove this API.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98426



More information about the llvm-commits mailing list