[PATCH] D63084: [llvm-readobj] Don't abort printing of dynamic table if string reference is invalid

[James Henderson via Phabricator via llvm-commits]

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