[PATCH] D57198: [llvm-objcopy] Implement --set-section-flags.

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/test/tools/llvm-objcopy/ELF/set-section-flags-and-rename.test:16
+    Flags:           [ ]
+    Content:        "c3c3c3c3"
+
----------------
Nit: get rid of the content. It's not needed. Can you get rid of the Type and Flags fields too, or are they required?


================
Comment at: llvm/test/tools/llvm-objcopy/ELF/set-section-flags-multiple.test:16
+    Flags:           [ ]
+    Content:        "c3c3c3c3"
+  - Name:            .bar
----------------
Ditto, here and below.


================
Comment at: llvm/test/tools/llvm-objcopy/ELF/set-section-flags.test:54
+    Flags:           [ ]
+    Content:        "c3c3c3c3"
+
----------------
Ditto.


================
Comment at: llvm/tools/llvm-objcopy/CopyConfig.cpp:341-344
+    if (!StringRef(Arg->getValue()).contains('='))
+      error("Bad format for --set-section-flags: missing '='");
+    auto Section2Flags = StringRef(Arg->getValue()).split('=');
+    uint64_t NewFlags = parseSetSectionFlagValue(Section2Flags.second);
----------------
This should look almost the same as the --rename-section code above, i.e. where the error checking for bad format etc is done.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D57198