[PATCH] D66409: [llvm-objcopy][MachO] Implement -Obinary

[George Rimar via Phabricator via llvm-commits]

grimar added inline comments.


================
Comment at: llvm/tools/llvm-objcopy/MachO/MachOWriter.cpp:503
+  // Sort sections by their addresses.
+  for (LoadCommand &LC : O.LoadCommands)
+    for (Section &Sec : LC.Sections)
----------------
`LC` -> `Cmd`?


================
Comment at: llvm/tools/llvm-objcopy/MachO/MachOWriter.cpp:518
+      Sec->Addr -= AddrBase;
+  }
+
----------------
This can be:

```
// Remove ....
for (Section *Sec : OrderedSections)
  Sec->Addr -= OrderedSections[0]->Addr;
```


================
Comment at: llvm/tools/llvm-objcopy/MachO/MachOWriter.cpp:531
+    return E;
+  memset(B.getBufferStart(), 0, TotalSize);
+
----------------
Does it make sense to remove this in favor of speed?
I.e. instead of filling the whole buffer with zeroes you can fill only gaps.
This might be better for large binaries.


================
Comment at: llvm/tools/llvm-objcopy/MachO/MachOWriter.h:72
+  Object &O;
+  Buffer &B;
+  MachOLayoutBuilder LayoutBuilder;
----------------
What do you think of using probably more traditional/clear `Obj` and `Buf` names?



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66409