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

Rafael Ávila de Espíndola rafael.espindola at gmail.com
Wed Jul 31 12:05:32 PDT 2013



================
Comment at: include/llvm/Object/ELF.h:113
@@ -553,1 +112,3 @@
 
+    uintX_t getEntSize() const { return EntitySize; }
+
----------------
nit: Which entities can be bigger than 4GB each? Maybe just using unsigned for EntitySize would do.


================
Comment at: include/llvm/Object/ELF.h:365
@@ -786,4 +364,3 @@
 
-  Elf_Rel_Iter endELFRel(const Elf_Shdr *sec) const {
-    return Elf_Rel_Iter(sec->sh_entsize, (const char *)
-                        (base() + sec->sh_offset + sec->sh_size));
+  Elf_Rel_Iter end_rel(const Elf_Shdr *sec) const {
+    return Elf_Rel_Iter(sec->sh_entsize,
----------------
Having ELF in the name of this methods would be redundant, so I agree with dropping it, but why the switch away from CamelCase?

================
Comment at: include/llvm/Object/ELF.h:386
@@ -815,3 +385,3 @@
   uint64_t getNumSections() const;
-  uint64_t getStringTableIndex() const;
+  uintX_t getStringTableIndex() const;
   ELF::Elf64_Word getSymbolTableIndex(const Elf_Sym *symb) const;
----------------
Nit: maybe an unsigned would do? With 64 bytes per section, the file would be 256 GB even if all 2^32 sections were empty.


================
Comment at: include/llvm/Object/ELF.h:1405
@@ +1404,3 @@
+  //       stripped.
+#if 0
+  for (Elf_Dyn_Iter DynI = begin_dynamic_table(), DynE = end_dynamic_table();
----------------
no #if 0 code please.

================
Comment at: lib/Object/ELFObjectFile.cpp:18
@@ -17,1 +17,3 @@
 namespace llvm {
+namespace object {
+#define LLVM_ELF_SWITCH_RELOC_TYPE_NAME(enum)                                  \
----------------
This is now duplicated from the header, no? I would be really nice to keep just one copy out of header for this!


================
Comment at: test/Object/corrupt.test:1
@@ +1,2 @@
+RUN: llvm-readobj %p/Inputs/corrupt.elf-x86-64 -sections \
+RUN:     2>&1 | FileCheck %s
----------------
llvm-readobject is not returning an error code? You will need to add a not before llvm-readobj now that we are using pipefail.

Please document in which way this file is corrupt. Please make sure we have corrupt tests for:

* getSymbolName
* getSymbolVersion
* getSectionName
* getSectionContents



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



More information about the llvm-commits mailing list