[PATCH] D42516: [llvm-objcopy] Add support for large indexes

James Henderson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Feb 22 01:53:44 PST 2018


jhenderson added a comment.

Code looks okay to me, aside from a couple of comment nits. I'll hold off approving though, since I'd like to see the other tests.



================
Comment at: tools/llvm-objcopy/Object.cpp:875
+    // If the section name string table section index is greater than or equal
+    // to
+    // SHN_LORESERVE (0xff00), this member has the value SHN_XINDEX (0xffff) and
----------------
Weird wrapping again?


================
Comment at: tools/llvm-objcopy/Object.cpp:1131
+  if (NeedsLargeIndexes) {
+    // This means we definitely need to have a section index table but if
+    // we already have one then we should use it instead of making a new
----------------
Have you wrapped slightly early here? I think we can get this comment down to 2 lines.


Repository:
  rL LLVM

https://reviews.llvm.org/D42516





More information about the llvm-commits mailing list