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

David Blaikie via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sat Feb 20 11:12:44 PST 2021


dblaikie accepted this revision.
dblaikie added a subscriber: rnk.
dblaikie added a comment.
This revision is now accepted and ready to land.

Generally looks OK to me. Though would be good to have @jhenderson, @rupprecht, and maybe @rnk (for the COFF suport) take a look too.

(this is closer to how LLVM emits objects (it uses a raw_pwrite_stream, so it can go back to write the header since it doesn't compute the total layout/length up-front) but a bit further from how lld emits its output (it uses the llvm::FileOutputBuffer which is underneath the Buffer this patch moves away from) - which is a bit awkward, but at least maybe it's less of a 3rd way of doing things... maybe. Wolud be good to try to unify all these things some day)



================
Comment at: llvm/tools/llvm-objcopy/llvm-objcopy.cpp:95
+static Expected<ArrayRef<uint8_t>>
+findBuildID(const CopyConfig &Config, const object::ELFFile<ELFT> &In) {
+  auto PhdrsOrErr = In.program_headers();
----------------
Might be easier to review if this code was moved around in a separate preparatory commit. So the code changes would be separate from the code moving.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91028



More information about the llvm-commits mailing list