[PATCH] D68575: [llvm-readobj][xcoff] implement parsing overflow section header.

[Hubert Tong via Phabricator via llvm-commits]

hubert.reinterpretcast added inline comments.


================
Comment at: llvm/tools/llvm-readobj/XCOFFDumper.cpp:58
+  static constexpr unsigned SectionFlagsTypeMask = 0xffffu;
   const XCOFFObjectFile &Obj;
 };
----------------
Add a blank line here. Also, I am wondering if this should be part of `llvm/BinaryFormat/XCOFF.h` (perhaps in `SectionHeader32`, or in a base class thereof when 64-bit support lands).


================
Comment at: llvm/tools/llvm-readobj/XCOFFDumper.cpp:455
+    case XCOFF::STYP_TYPCHK:
+      // TODO : The interpretation of loader, exception, type check section
+      // headers are different from that of generic section header. We will
----------------
The "TODO" still has a colon surrounded by spaces on both sides after it. I do not think that we have been using colons after "TODO".

Still missing "and" before "type check section headers".

Still missing "s" after "generic section header".

Typo "seciton" is still present.


================
Comment at: llvm/tools/llvm-readobj/XCOFFDumper.cpp:463
+    }
+    // For now we just dump the section type flags.
+    if (SectionType & SectionFlagsReservedMask)
----------------
Suggestion: "For now we just dump the section type portion of the flags."


Repository:
  rL LLVM

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

https://reviews.llvm.org/D68575