[PATCH] D117281: [llvm-objcopy][MachO] Implement --update-section

Alexander Shaposhnikov via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sat Jan 22 18:17:38 PST 2022


alexander-shaposhnikov added inline comments.


================
Comment at: llvm/tools/llvm-objcopy/MachO/MachOObjcopy.cpp:343
+                                          &Obj]() -> Expected<Section &> {
+    if (!SecName.contains(','))
+      return findSection(SecName, Obj);
----------------
we probably don't need 2 cases here (specified / unspecified segment name),
for consistency it would be better to always require the full name, e.g. __TEXT,__text
(`otool -d` also understands this notation, llvm-objcopy --add-section uses it as well)


================
Comment at: llvm/tools/llvm-objcopy/MachO/MachOObjcopy.cpp:445
+  for (const auto &Flag : Config.UpdateSection) {
+    std::pair<StringRef, StringRef> SectionAndContents = Flag.split("=");
+    if (Error E = updateSection(SectionAndContents.first,
----------------
nit:
```
std::tie(SectionName, FileName) = Flag.split('=');
```


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

https://reviews.llvm.org/D117281



More information about the llvm-commits mailing list