[PATCH] [Object] Split the ELF interface into 3 parts.

Sean Silva silvas at purdue.edu
Mon Jun 24 18:42:23 PDT 2013


  A couple "peephole" comments. Looks pretty good though. (I'll get to a full review tomorrow)


================
Comment at: include/llvm/Object/ELFObjectFile.h:747-751
@@ +746,7 @@
+
+template <class ELFT>
+StringRef ELFObjectFile<ELFT>::getRelocationTypeName(uint32_t Type) const {
+  StringRef Res = "Unknown";
+  switch (EF.getHeader()->e_machine) {
+  case ELF::EM_X86_64:
+    switch (Type) {
----------------
This huge switch probably should be in a static (non-templated) helper function to avoid duplicating this lookup table.

================
Comment at: include/llvm/Object/ELFObjectFile.h:742-745
@@ +741,6 @@
+
+#define LLVM_ELF_SWITCH_RELOC_TYPE_NAME(enum)                                  \
+  case ELF::enum:                                                              \
+  Res = #enum;                                                                 \
+  break;
+
----------------
Can you just directly `return #enum`?

================
Comment at: include/llvm/Object/ELF.h:1235
@@ +1234,3 @@
+template <class ELFT>
+uint64_t ELFFile<ELFT>::getStringTableIndex() const {
+  if (Header->e_shnum == ELF::SHN_UNDEF) {
----------------
Should this be `uintX_t`?


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



More information about the llvm-commits mailing list