[PATCH] D69526: [llvm-objcopy] Address post-commit reviews of D69093

Tue Oct 29 00:40:05 PDT 2019

grimar accepted this revision.
grimar added a comment.
This revision is now accepted and ready to land.

LGTM with a nit, put please wait for others too.

================
Comment at: llvm/test/tools/llvm-objcopy/ELF/add-symbol-no-symtab.test:74
+    Type: SHT_STRTAB
+Symbols:
+...
----------------
I'd suggest to use "Symbols: []" to emphasise we just want to create a ".symtab". Up to you.

================
Comment at: llvm/tools/llvm-objcopy/ELF/Object.cpp:1544
+            "e_shstrndx field value " + Twine(ShstrIndex) + " in elf header " +
+                " is not a string table");
+
----------------
I see you just moved this part, so comment below is FTR.
A field value can never be a string table.
"does not reference a string table" would probably be better.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69526