[PATCH] D49979: [llvm-objcopy] Add --dump-section

[James Henderson via Phabricator via llvm-commits]

jhenderson accepted this revision.
jhenderson added a comment.
This revision is now accepted and ready to land.

LGTM, with the suggested changes, but please get @jakehehrlich to confirm he's happy with the test changes.



================
Comment at: test/tools/llvm-objcopy/dump-section.test:42
+
+#NON-ALLOC: 0000000 ca fe
+
----------------
Nit: Move this to the line immediately after the earlier "CHECK".


================
Comment at: tools/llvm-objcopy/llvm-objcopy.cpp:246
+      if (Sec.OriginalData.size() == 0)
+        return make_error<StringError>("can't dump section [" + SecName +
+                                           "] - it has no contents",
----------------
I think this is a little unusual formatting of an error message, using '[' and '-'. I think a more common style would be to use '"' and ':':

'Can't dump section ".foo": it has no contents'

Also, llvm-objcopy's style is generally to use upper-case for the first letter in the error message.


Repository:
  rL LLVM

https://reviews.llvm.org/D49979