[PATCH] D58763: [llvm-objdump] Should print unknown d_tag in hex format
James Henderson via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Feb 28 01:58:31 PST 2019
jhenderson added inline comments.
================
Comment at: tools/llvm-objdump/ELFDump.cpp:179
StringRef Str = StringRef(Elf->getDynamicTagAsString(Dyn.d_tag));
----------------
Higuoxing wrote:
> grimar wrote:
> > Should getDynamicTagAsString just return `nullptr` on unknown flag maybe?
> > (It seems used only once in LLVM, for the place below, btw)
> >
> > @jhenderson, what do you think?
> There are several functions that return "unknown". I am not so confident to change those codes in `/lib`. In my opinion, I think those should return `""`.
I'd say neither is the right thing to do. I actually think just printing the hex number is not good enough. I think it should be something like `<unknown:>0x12345678`, and I would highly recommend doing that inside getDynamicTagAsString(). It seems rather odd to be comparing against "unknown". The alternative would be to change the interface to return an Optional<> instead of a string, but the name of the function just makes me think I wouldn't need to do anything with the output at all.
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D58763/new/
https://reviews.llvm.org/D58763
More information about the llvm-commits
mailing list