[PATCH] D32609: Update llvm-readobj -coff-resources to display tree structure.

[Rui Ueyama via Phabricator via llvm-commits]

ruiu added a comment.

In general you want to avoid checking in a binary file. Can you create that object file using llvm-mc? If so, please check in an assembly file or a .c file and generate an object file when the test is executed.



================
Comment at: llvm/include/llvm/Object/COFF.h:641-644
+  union {
+    support::ulittle32_t DataEntryOffset;
+    support::ulittle32_t SubdirOffset;
+  } Offset;
----------------
You are not using this class, are you? If you plan to use it later, please introduce this later.


================
Comment at: llvm/include/llvm/Object/COFF.h:1068-1071
+  std::error_code getTableAtOffset(uint32_t Offset,
+                                   const coff_resource_dir_table *&Table);
+  std::error_code getDirStringAtOffset(uint32_t Offset,
+                                       StringRef &DirString) const;
----------------
I think it is better to use ErrorOr<T>.


================
Comment at: llvm/include/llvm/Object/COFF.h:1076
+    case 1:
+      return StringRef("kRT_CURSOR (ID 1)");
+    case 2:
----------------
StringRef has a (non-explicit) constructor that takes char*, so you don't need to construct StringRef explicitly. You can just return a string literal.


https://reviews.llvm.org/D32609