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

[Jordan Rupprecht via Phabricator via llvm-commits]

rupprecht 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}}},
----------------
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"?


================
Comment at: llvm/tools/llvm-objcopy/CopyConfig.cpp:457-462
+  if (InputFormat.empty())
+    Config.InputFormat = FileFormat::Unspecified;
+  else if (InputFormat == "ihex")
+    Config.InputFormat = FileFormat::IHex;
+  else if (InputFormat == "binary") {
+    Config.InputFormat = FileFormat::Binary;
----------------
This should use a `StringSwitch`

(I suppose adding an `Unrecognized` file format type to handle the 'else' part?)


================
Comment at: llvm/tools/llvm-objcopy/CopyConfig.cpp:473-475
+    return createStringError(errc::invalid_argument,
+                             "unsupported input format: '%s'",
+                             InputFormat.str().c_str());
----------------
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
```


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

https://reviews.llvm.org/D63239