[PATCH] D113356: [llvm-objdump] -p: Dump PE header for PE/COFF

[Alexander Shaposhnikov via Phabricator via llvm-commits]

alexander-shaposhnikov added inline comments.


================
Comment at: llvm/tools/llvm-objdump/COFFDump.cpp:42
+public:
+  COFFDumper(const llvm::object::COFFObjectFile *Obj) : Obj(Obj) {
+    if (Obj->getPE32Header())
----------------
explicit
maybe pass const ref ? (e.g. nullptr is not a valid value for the argument)


================
Comment at: llvm/tools/llvm-objdump/COFFDump.cpp:51
+  template <typename T, typename TEnum>
+  StringRef getEnumName(T Value, ArrayRef<EnumEntry<TEnum>> EnumValues) const {
+    for (const EnumEntry<TEnum> &EnumItem : EnumValues)
----------------
where is this method used ?


================
Comment at: llvm/tools/llvm-objdump/COFFDump.cpp:63
+  template <typename T, typename TEnum>
+  void printOptionalEnumName(T Value,
+                             ArrayRef<EnumEntry<TEnum>> EnumValues) const {
----------------
this method does not appear to use Obj or Is64, make it static ?


================
Comment at: llvm/tools/llvm-objdump/COFFDump.cpp:81
+
+const EnumEntry<uint16_t> PEHeaderMagic[] = {
+    {uint16_t(COFF::PE32Header::PE32), "PE32"},
----------------
static constexpr  ?


================
Comment at: llvm/tools/llvm-objdump/COFFDump.cpp:166
+
+  static const char *DirName[16] = {
+      "Export Directory [.edata (or where ever we found it)]",
----------------
constexpr ?


================
Comment at: llvm/tools/llvm-objdump/COFFDump.cpp:185
+  outs() << "\nThe Data Directory\n";
+  for (uint32_t I = 0; I != 16; ++I) {
+    uint32_t Addr = 0, Size = 0;
----------------
maybe 16 can be replaced with a named constant or some expression ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113356