[PATCH] D92636: [llvm-readelf/obj] - Improve diagnostics when printing NT_FILE notes.

[Jordan Rupprecht via Phabricator via llvm-commits]

rupprecht added a comment.

Thanks!



================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:5338
+    return createError(
+        "unable to read the file mappings number: the note of size 0x" +
+        Twine::utohexstr(Desc.size()) + " is truncated");
----------------
This is checking more than just the file mappings number, it also includes the page size (note the check above is for 2 bytes).
How about: "the note of size 0x1 is too short, expected at least 0x2"?


================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:5349
+    return createError(
+        "unable to read start/end/offset tripples: the note of size 0x" +
+        Twine::utohexstr(Desc.size()) + " is truncated");
----------------
I'd probably just say "unable to read file mappings".

If you prefer this, there's a typo: tripples -> triples.


================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:5350
+        "unable to read start/end/offset tripples: the note of size 0x" +
+        Twine::utohexstr(Desc.size()) + " is truncated");
 
----------------
Maybe this could also say how many bytes it expected it to be? It should be `(2 + 3 * FileCount) * Bytes` IIRC.

And maybe say what `FileCount` is? That may give indication to a developer debugging a bad note whether the note is really too short or if the number of files is bogus.


================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:5363
+      return createError(
+          "unable to read the file name for the file mapping with index " +
+          Twine(I) + ": the note of size 0x" + Twine::utohexstr(Desc.size()) +
----------------
"file name for the file mapping" feels a little redundant -- maybe just "file name for the mapping"?


================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:5576
+    for (const typename ELFT::Note Note : Obj.notes(P, Err)) {
+      ++Index;
+      if (Error E = ProcessNoteFn(Note))
----------------
`Index` should probably be incremented at the end, otherwise it reports the first note as index 1, not index 0.


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

https://reviews.llvm.org/D92636