[PATCH] D63395: [llvm-objcopy][MachO] Support load commands used in executables/shared libraries

[Alexander Shaposhnikov via Phabricator via llvm-commits]

alexshap added inline comments.


================
Comment at: llvm/tools/llvm-objcopy/MachO/MachOLayoutBuilder.cpp:61
+// Updates the index and the number of local/external/undefined symbols. Here we
+// assume that MLC is a LC_DYSYMTAB and the nlist entries in the symbol table
+// are already sorted by the those types.
----------------
I'd be defensive here: add an assert/logical check (for example, via std::is_sorted + custom comparator)
+ probably we probably need to actually sort at some point, don't we ?


================
Comment at: llvm/tools/llvm-objcopy/MachO/MachOLayoutBuilder.cpp:75
+  uint32_t NumExtDefSymbols = 0;
+  for (; Iter != End; Iter++) {
+    if (((*Iter)->n_type & MachO::N_TYPE) == MachO::N_UNDF)
----------------
nit: ++Iter


================
Comment at: llvm/tools/llvm-objcopy/MachO/MachOLayoutBuilder.cpp:79
+
+    NumExtDefSymbols++;
+  }
----------------
nit: ++NumExtDefSymbols


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

https://reviews.llvm.org/D63395