[PATCH] D53667: [llvm-objcopy] Handle -O <format> flag.

[Jordan Rupprecht via Phabricator via llvm-commits]

rupprecht added inline comments.


================
Comment at: test/tools/llvm-objcopy/ELF/cross-arch-headers.test:3
+
+# RUN: llvm-objcopy %t.o -Oelf32-i386 %t.elf32_i386.o
+# RUN: llvm-readobj -file-headers %t.elf32_i386.o | FileCheck %s --check-prefixes=CHECK,I386,32
----------------
jhenderson wrote:
> Let's put a space between -O and its argument to make it a little more readable and consistent with other tests.
Done here, but I do want to make sure that it works this way to ease migration, so I've left in a single test case (that also checks for no spaces in -Ibinary and -Barch) w/o the spaces.


================
Comment at: test/tools/llvm-objcopy/ELF/cross-arch-headers.test:4
+# RUN: llvm-objcopy %t.o -Oelf32-i386 %t.elf32_i386.o
+# RUN: llvm-readobj -file-headers %t.elf32_i386.o | FileCheck %s --check-prefixes=CHECK,I386,32
+
----------------
jhenderson wrote:
> Nit: -file-headers uses a single dash, but --check-prefixes uses two. Please be consistent (I'd prefer double, but that's just me).
Done + I'll fix the other test cases in this dir in a separate NFC commit


================
Comment at: test/tools/llvm-objcopy/ELF/cross-arch-sections-symbols.test:26
+      Section:  .text
+      Value:    0x1234
+    - Name:     bar
----------------
jhenderson wrote:
> These values should probably be inside the sections, once they have non-zero size.
Done; is this what you meant?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D53667