[PATCH] COFFDumper: Dump data directory entries.

Rui Ueyama ruiu at google.com
Mon Jul 15 20:41:51 PDT 2013



================
Comment at: test/tools/llvm-readobj/file-headers.test:111
@@ -112,1 +110,3 @@
+PE32-NEXT:   SectionCount: 3
+PE32-NEXT:   TimeDateStamp:
 PE32-NEXT:   PointerToSymbolTable: 0x0
----------------
Nico Rieck wrote:
> Please verify and include the new timestamp here because it's relevant to this test.
Done.

================
Comment at: lib/Object/COFFObjectFile.cpp:619
@@ +618,3 @@
+                                            const data_directory *&Res) const {
+  if (!DataDirectory || PE32Header->NumberOfRvaAndSize <= index)
+    return object_error::parse_failed;
----------------
Nico Rieck wrote:
> Small nit, but the second check stumped me for a moment. The "subject" variable usually seems to come first in LLVM code.
Done.

================
Comment at: tools/llvm-readobj/COFFDumper.cpp:567
@@ +566,3 @@
+  const data_directory *data;
+  Obj->getDataDirectory(index, data);
+  W.printHex(fieldName + "RVA", data->RelativeVirtualAddress);
----------------
Nico Rieck wrote:
> You should check the error code here and skip it with a diagnostic. Also, upper camelcase for parameters and "data".
In other places in this file, it just skips a field without a diagnostic if there's an error.

================
Comment at: tools/llvm-readobj/COFFDumper.cpp:63
@@ -62,1 +62,3 @@
 
+  void printDataDirectory(uint32_t index, std::string fieldName);
+
----------------
Nico Rieck wrote:
> Upper camelcase for the parameters.
Done.


http://llvm-reviews.chandlerc.com/D1148



More information about the llvm-commits mailing list