[PATCH] D66283: [llvm-objcopy][MachO] Implement --add-section

[Seiya Nuta via Phabricator via llvm-commits]

seiya marked 2 inline comments as done.
seiya added inline comments.


================
Comment at: llvm/tools/llvm-objcopy/MachO/Object.h:76-81
+  StringRef getContent() const {
+    if (OwnedContentData.size() > 0)
+      return StringRef(reinterpret_cast<const char *>(OwnedContentData.data()),
+                       OwnedContentData.size());
+    return Content;
+  }
----------------
rupprecht wrote:
> A common use of GNU objcopy is to run with `--add-section .note.GNU-stack=/dev/null`, which I imagine corresponds to a 0-sized section. I think you might want to wrap `OwnedContentData` in `Optional<>`. (Assuming a zero-sized section is valid in Mach-O). Additionally, can you add a test for --add-section w/ /dev/null?
> 
> The ELF code doesn't use `Optional` because it takes a different approach by having subclasses for different types of sections, so `OwnedDataSection` is different from `Section` -- I don't know if that makes sense here
Replaced with Optional and added a test case for `/dev/null`.


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

https://reviews.llvm.org/D66283