[PATCH] D63843: [Object][XCOFF] Add support for 64-bit file header and section header dumping.

[Hubert Tong via Phabricator via llvm-commits]

hubert.reinterpretcast added inline comments.


================
Comment at: llvm/lib/Object/XCOFFObjectFile.cpp:187
 
-Expected<StringRef> XCOFFObjectFile::getSectionName(DataRefImpl Sec) const {
-  const char *Name = toSection(Sec)->Name;
-  auto NulCharPtr =
-      static_cast<const char *>(memchr(Name, '\0', XCOFF::SectionNameSize));
-  return NulCharPtr ? StringRef(Name, NulCharPtr - Name)
-                    : StringRef(Name, XCOFF::SectionNameSize);
+ Expected<StringRef> XCOFFObjectFile::getSectionName(DataRefImpl Sec) const {
+  return generateStringRef(getSectionNameInternal(Sec), XCOFF::SectionNameSize);
----------------
Remove the space before the return type.


================
Comment at: llvm/lib/Object/XCOFFObjectFile.cpp:386
+  default:
+    Expected<DataRefImpl> SI = getSectionByNum(SectionNum);
+    if (SI)
----------------
What does `SI` mean?


================
Comment at: llvm/lib/Object/XCOFFObjectFile.cpp:475
+  // If there is a string table, then the buffer must contain at least 4 bytes
+  // for the string tables size. Not having as string table is not an error.
+  if (Offset + 4 > Obj->Data.getBufferSize())
----------------
Use either "string table's size" or "string-table size".
Typo: s/as string/a string/;



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63843