[PATCH] D60784: [Object][XCOFF] Add intial support for parsing/dumping section header table.

Sean Fertile via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Apr 18 12:46:58 PDT 2019


sfertile marked 8 inline comments as done and an inline comment as not done.
sfertile added inline comments.


================
Comment at: llvm/include/llvm/Object/XCOFFObjectFile.h:24
 #include "llvm/Object/ObjectFile.h"
 #include "llvm/Object/SymbolicFile.h"
 #include "llvm/Support/Casting.h"
----------------
sfertile wrote:
> DiggerLin wrote:
> > ObjectFile.h already include SymbolicFile.h , I think there do need SymbolicFile.h here. and also 
> > #include "llvm/Object/Binary.h"
> > #include "llvm/Object/Error.h"
> > #include "llvm/BinaryFormat/Magic.h"
> > #include "llvm/ADT/iterator_range.h"
> > 
> Good point, most of these are already included from ObjectFile.h. I'll clean up our header includes but I'll land that in a separate NFC patch.
Didn't intend to mark this done yet.


================
Comment at: llvm/lib/Object/XCOFFObjectFile.cpp:310
+  if (getNumberOfSections() != 0) {
+    if (!checkSize(Data, EC, CurPtr,
+                   getNumberOfSections() * getSectionHeaderSize()))
----------------
hubert.reinterpretcast wrote:
> `getObject` (called below) takes a `Size` parameter. It can be used to check for the right number of sections without this call to `checkSize`.
I've also removed 'checkSize' as it no longer has any uses.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60784





More information about the llvm-commits mailing list