[PATCH] D65346: [llvm-objcopy] Improve --add-section argument string parsing

[Sergey Dmitriev via Phabricator via llvm-commits]

sdmitriev marked 7 inline comments as done.
sdmitriev added inline comments.


================
Comment at: llvm/test/tools/llvm-objcopy/COFF/add-section.test:44
 
-# CHECK-ERR2: llvm-objcopy{{(.exe)?}}: error: '[[FILE]]': bad format for --add-section
+# CHECK-ERR2: llvm-objcopy{{(.exe)?}}: error: bad format for --add-section: missing '='
+
----------------
grimar wrote:
> If you do not report the file name anymore then you do not need `-DFILE=%t`
Right.


================
Comment at: llvm/test/tools/llvm-objcopy/ELF/add-section.test:41
+## to be added does not exist.
+# RUN: not llvm-objcopy --add-section=.section.name=%t.missing %t %t.out 2>&1 | FileCheck -DFILE=%t -DFILE1=%t.missing %s --check-prefixes=CHECK-ERR1
+
----------------
grimar wrote:
> Perhaps `DFILE1` and `DFILE2` would be better when you have 2 files.
> 
> I'd also suggest renaming `CHECK-ERR*` -> `ERR*`.
Done.


================
Comment at: llvm/test/tools/llvm-objcopy/ELF/add-section.test:53
+
+# CHECK-ERR3: llvm-objcopy{{(.exe)?}}: error: bad format for --add-section: missing file name
----------------
grimar wrote:
> FWIW I'd omit `llvm-objcopy{{(.exe)?}}:` part here and above. It doesn't seem useful actually to test.
Done.


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

https://reviews.llvm.org/D65346