[PATCH] D118189: [llvm-objcopy][COFF] Implement --update-section

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/test/tools/llvm-objcopy/COFF/update-section.test:16
+
+# RUN: llvm-objcopy --update-section=.text=%t.smaller %t - | llvm-readobj -S -x .text - | FileCheck %s --check-prefix=SMALLER
+# SMALLER:      Section {
----------------
Just confirming that it is deliberate to keep the section size the same, padded with 0? For ELF, the section is shrunk, which feels more like what I'd expect.


================
Comment at: llvm/test/tools/llvm-objcopy/COFF/update-section.test:25-26
+
+# RUN: llvm-objcopy --update-section=.text=%t.diff --update-section=.other=%t.diff %t - \
+# RUN: | llvm-readobj -S -x .text -x .other - | FileCheck %s --check-prefix=MULTIPLE
+# MULTIPLE:     Section {
----------------
I have a minor preference for the suggested edit style for flowing over multiple lines.


================
Comment at: llvm/test/tools/llvm-objcopy/COFF/update-section.test:49
+# RUN: not llvm-objcopy --update-section=.text=%t.noexist %t /dev/null 2>&1 | FileCheck %s --check-prefix=NOENT
+# NOENT: error: {{.*}}no such file or directory
+
----------------
If this is a system error message, there may be a between different OS versions. I believe lit now has substitutions for common error messages to handle this. Look for "%errc_ENOENT" for example usage.


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

https://reviews.llvm.org/D118189