[PATCH] D81527: [llvm-install-name-tool] Add `delete_rpath` option

Alexander Shaposhnikov via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jun 15 22:32:29 PDT 2020


alexshap added inline comments.


================
Comment at: llvm/tools/llvm-objcopy/MachO/Object.cpp:42
+  return Error::success();
+}
+
----------------
alexshap wrote:
> Object stores indices of some special load commands which need to be updated (in this method).
> 
>    for (const LoadCommand &LC : LoadCommand) {
>         switch (LC.cmd) {
>            ...
>         }
>    }
> 
i see, thanks, almost there. A few suggestions from me:

1. I think the quadratic behavior here is unnecessary, I would simply use smth like
      
      for (size_t Index = 0, Size = LoadCommands.size(); Index < Size; ++Index) {
           ...
      }

2. Let's factor out it into a helper method, e.g. updateLoadCommandsIndexes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81527





More information about the llvm-commits mailing list