[PATCH] D63239: [llvm-objcopy][NFC] Refactor output target parsing

[Seiya Nuta via Phabricator via llvm-commits]

seiya marked 2 inline comments as done.
seiya added inline comments.


================
Comment at: llvm/tools/llvm-objcopy/CopyConfig.cpp:288
     // x86
-    {"elf32-i386", {ELF::EM_386, false, true}},
-    {"elf32-x86-64", {ELF::EM_X86_64, false, true}},
-    {"elf64-x86-64", {ELF::EM_X86_64, true, true}},
+    {"elf32-i386", {FileFormat::ELF, {ELF::EM_386, false, true}}},
+    {"elf32-x86-64", {FileFormat::ELF, {ELF::EM_X86_64, false, true}}},
----------------
rupprecht wrote:
> I haven't looked at the full list of bfd targets (for macho/coff in particular), but do we need the extra field, or can we infer it from starting with "elf"?
I'm not quite sure but according to my GNU objcopy build (with `--enable-targets=all`) you're right: we can infer the file type from the prefix.


================
Comment at: llvm/tools/llvm-objcopy/CopyConfig.cpp:473-475
+    return createStringError(errc::invalid_argument,
+                             "unsupported input format: '%s'",
+                             InputFormat.str().c_str());
----------------
rupprecht wrote:
> I'm not sure this is really an error. Behold:
> 
> ```
> $ echo 'int x=0;' | ~/dev/clang -x c -c - -o /tmp/bss.o
> $ objcopy -Ielf32-x86-64 /tmp/bss.o /tmp/bss2.o
> $ file /tmp/bss.o /tmp/bss2.o
> /tmp/bss.o:  ELF 64-bit LSB relocatable, x86-64, version 1 (SYSV), not stripped
> /tmp/bss2.o: ELF 32-bit LSB relocatable, x86-64, version 1 (SYSV), not stripped
> ```
Nice catch! Will fix this.


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

https://reviews.llvm.org/D63239