[PATCH] D63084: [llvm-readobj] Don't abort printing of dynamic table if string reference is invalid
James Henderson via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Jun 13 01:29:30 PDT 2019
jhenderson accepted this revision.
jhenderson added a comment.
Looks good, with the suggested changes to the if statements.
================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:1942
+ OS << TagNames.at(Type) << ": ";
+ if (DynamicStringTable.size() == 0)
+ OS << "<String table is empty or was not found> ";
----------------
ychen wrote:
> MaskRay wrote:
> > DynamicStringTable.empty()
> done
Tip: you can mark individual comments as "Done" by clicking the "Done" button in the comment box header. I find it quite useful when reviewing code if that has been done.
================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:1794
+ else if (Value < DynamicStringTable.size()) {
+ if (WithBracket) OS << "[";
+ OS << StringRef(DynamicStringTable.data() + Value);
----------------
LLVM standard has the body of an if on a new line, I believe, i.e.
```
if (WithBracket)
OS << "[";
```
================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:1796
+ OS << StringRef(DynamicStringTable.data() + Value);
+ if (WithBracket) OS << "]";
+ } else
----------------
Ditto
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D63084/new/
https://reviews.llvm.org/D63084
More information about the llvm-commits
mailing list