[PATCH] D64674: [llvm-readobj] Refactor dynamic string table indexing into a function.

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: llvm/test/tools/llvm-readobj/elf-dynamic-malformed.test:172
+# BAD-STRTAB-LLVM: LoadName: <String table is empty or was not found>
+# BAD-STRTAB-LLVM: 0x0000000000000001  NEEDED   Shared library: [<String table is empty or was not found>]
+# BAD-STRTAB-GNU:  0x0000000000000001 (NEEDED)  Shared library: [<String table is empty or was not found>]
----------------
grimar wrote:
> ychen wrote:
> > grimar wrote:
> > > Isn't it inconsistent? For line above you print `<error>`, but here you print `[<error>]`.
> > > Was it intentional change? (it is not NFC then).
> > Title updated to reflect that it is not NFC.
> So, previously we printed `[valid string]` and `<error text>`.
> After this patch can print `[valid string]`, `[<error text>]` and `<error text>`. 
> What is not very consistent. Why the new behavior is desired?
> 
> @jhenderson, what do you think?
I'm fine with either choice (though I have a slight preference for non-capitalized messages)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64674