[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