[PATCH] D91028: [llvm-objcopy][NFC] replace class Buffer/MemBuffer/FileBuffer with streams.

[Alexey Lapshin via Phabricator via llvm-commits]

avl added a comment.

Will address all comments. I have a two questions/comments inside...



================
Comment at: llvm/tools/llvm-objcopy/MachO/MachOObjcopy.cpp:466
+
+    SmallVector<char, 0> Buffer;
+    raw_svector_ostream MemStream(Buffer);
----------------
jhenderson wrote:
> I'm not sure if it ever happened, but there was some discussion on the mailing lists about making `0` the default value if the size wasn't specified. Please could you check whether you need this second parameter still.
There is a comment for SmallVector that recommended practice is to not specify default value if there is no well motivated choice. Then it would automatically select one. In this case I think we do not need any stack data. So I specified 0 to avoid stack allocation, which looks correct in this case.


================
Comment at: llvm/tools/llvm-objcopy/llvm-objcopy.cpp:407-410
+    if (Error E = writeToFile(
+            Config.OutputFilename, [&](raw_ostream &OutFile) -> Error {
+              return ProcessRaw(Config, *BufOrErr->get(), OutFile);
+            }))
----------------
jhenderson wrote:
> I don't understand this change? This seems to be doing something quite different to what it was doing before.
It looks like it does the same. Originally, there were created a file called Config.OutputFilename and ProcessRaw put data in it.  In this patch writeToFile helper creates a file called Config.OutputFilename and calls ProcessRaw to store data into it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91028