[PATCH] D89071: [SystemZ/z/OS] Add GOFFObjectFile class and details of GOFF file format

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/lib/Object/GOFFObjectFile.cpp:86
+      //   - where the ED is zero length but
+      //     contains a label (LD)
+      GOFF::ESDSymbolType SymbolType;
----------------
(reminder - comments must end in a full stop)


================
Comment at: llvm/lib/Object/GOFFObjectFile.cpp:341
+
+// SectionRef
+const uint8_t *GOFFObjectFile::getSectionEdEsdRecord(DataRefImpl &Sec) const {
----------------
When I said the following to `SymbolRef` above:

> These sorts of comments don't add anything, in my opinion. Just delete them (the function names describe things sufficiently).


That wasn't referring to just the one comment. Please delete all of these sort of comments.


================
Comment at: llvm/lib/Object/GOFFObjectFile.cpp:397
+    const char *ChrPtr = reinterpret_cast<const char *>(RldRecord);
+    RelocationData.append(ChrPtr + 6, AppendLength); // copy from initial record
+    Remainder -= AppendLength;
----------------
jhenderson wrote:
> 
Please address both edits suggested in the previous comment, not just the second one.


================
Comment at: llvm/lib/Object/GOFFObjectFile.cpp:404
+                         : (GOFF::RecordLength - 3);
+      RelocationData.append(ChrPtr + 3, AppendLength); // copy from continuation
+      Remainder -= AppendLength;
----------------
jhenderson wrote:
> 
Ditto.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89071