[PATCH] D53311: [llvm-objcopy] Introduce dispatch mechanism based on input type

Jake Ehrlich via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Oct 23 11:32:30 PDT 2018


jakehehrlich added a comment.

I think I just want an `executeObjcopyOnRawBinary` dispatch function. Everything else looks good to me here.



================
Comment at: llvm-objcopy.cpp:655
     FileBuffer FB(Config.OutputFilename);
-    BinaryReader Reader(Config.BinaryArch, BufOrErr->get());
-    executeElfObjcopyOnBinary(Config, Reader, FB,
-                              getOutputElfType(Config.BinaryArch));
+    elf::executeObjcopyOnRawBinary(Config, *BufOrErr->get(), FB);
   } else {
----------------
jhenderson wrote:
> It seems a bit odd still having mention of elf at this level. Is it allowed to read binary input for COFF or Mach-O output?
I believe it should be possible for both of those. There should be a second dispatch on the output format when that happens. Right now we don't have all the -O/-I stuff hooked up in a general way just yet. It seems a bit out of scope to do that in this change. We should add a check to make sure that the output format is ELF here. I think I'd like a function that mirrors executeObjcopyOnBinary but for raw binaries.


================
Comment at: llvm-objcopy.cpp:665
     } else {
       FileBuffer FB(Config.OutputFilename);
+      executeObjcopyOnBinary(Config, *BinaryOrErr.get().getBinary(), FB);
----------------
to mirror executeObjcopyOnArchive, can we move the construction of the file buffer into executeObjcopyOnBinary.


Repository:
  rL LLVM

https://reviews.llvm.org/D53311





More information about the llvm-commits mailing list