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

[Seiya Nuta via Phabricator via llvm-commits]

seiya added inline comments.


================
Comment at: llvm/tools/llvm-objcopy/MachO/MachOObjcopy.cpp:80
+  StringRef TargetSegName = Pair.first;
+  Section Sec(TargetSegName, SecName);
+  Sec.setOwnedContentData(Content);
----------------
rupprecht wrote:
> `SecName` is actually the pair of <segment>,<section>, so shouldn't `SecName` be replaced with `Pair.second`? I think the test case you have shows this.
Oops, I didn't noticed that. Thanks!


================
Comment at: llvm/tools/llvm-objcopy/MachO/Object.cpp:44-46
+    return StringRef(MLC.segment_command_data.segname,
+                     strnlen(MLC.segment_command_data.segname,
+                             sizeof(MLC.segment_command_data.segname)));
----------------
rupprecht wrote:
> Is there a utility you could extract to convert from possibly-null-delimited segment/section names to StringRefs? This pattern is used above with the strncpy and also in MachOReader
Added `extractSegmentName` for this. I didn't updated `strncpy` above because it's for filling the segment name, not extracting a possibly-null-terminated string.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66283