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

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: tools/llvm-objcopy/ObjcopyOpts.td:102
+                   MetaVarName<"section=file">,
+                   HelpText<"Dump content of section named <section> into file <file>.">;
----------------
Nit: content -> contents

Also, it looks like we're being inconsistent with whether help text should have trailing full stops. I think the majority has none, so you might want to remove it here, and we can do a NFC tidy up later to bring the rest into line with each other (I could easily be persuaded that actually they should all have them - we should aim for consistency with other parts of LLVM).


================
Comment at: tools/llvm-objcopy/llvm-objcopy.cpp:475
+  if (!Config.DumpSection.empty()) {
+    for (const auto &Flag : Config.DumpSection) {
+      auto SecPair = Flag.split("=");
----------------
jakehehrlich wrote:
> You can just iterate over these instead of checking.
To be fair, this is the same as how the other options have been implemented.


================
Comment at: tools/llvm-objcopy/llvm-objcopy.cpp:477-478
+      auto SecPair = Flag.split("=");
+      auto SecName = SecPair.first;
+      auto File = SecPair.second;
+      if (ObjectFile *O = dyn_cast<ObjectFile>(Reader.Bin)) {
----------------
I'd prefer less use of auto around here.


Repository:
  rL LLVM

https://reviews.llvm.org/D49979