[PATCH] D17523: llvm-readobj: enable GNU output style sections and relocations printing for ELF files

[khemant@codeaurora.org via llvm-commits]

khemant added inline comments.

================
Comment at: tools/llvm-readobj/ELFDumper.cpp:47
@@ -46,1 +46,3 @@
 
+#define DEFINE_ELF_TYPES                                                       \
+  typedef ELFFile<ELFT> ELFO;                                                  \
----------------
Bigcheese wrote:
> I would prefer this as:
> 
> #define TYPEDEF_ELF_TYPES(ELFT) ...
> 
> So that it's clear that it's a typedef and what it depends on.
Got it. Will do this.

================
Comment at: tools/llvm-readobj/ELFDumper.cpp:272
@@ +271,3 @@
+    Field &operator=(StringRef S) {
+      this->Str = S;
+      return *this;
----------------
Bigcheese wrote:
> Don't need this->
Will do.

================
Comment at: tools/llvm-readobj/ELFDumper.cpp:523
@@ -459,3 +522,3 @@
                                             const Elf_Sym *symb,
-                                            bool &IsDefault) {
+                                            bool &IsDefault) const {
   // This is a dynamic symbol. Look in the GNU symbol version table.
----------------
echristo wrote:
> Can you do this separately?
This change was needed due to series of const functions that compiler figured out when I make the pointer to ELFDumper in the DumpStyle class a const pointer.

ELFDumper.cpp:521:11: note:   no known conversion for implicit 'this' parameter from 'const {anonymous}::ELFDumper<llvm::object::ELFType<(llvm::support::endianness)0u, true> >* const' to '{anonymous}::ELFDumper<llvm::object::ELFType<(llvm::support::endianness)0u, true> >*'


================
Comment at: tools/llvm-readobj/ELFDumper.cpp:572
@@ -508,3 +571,3 @@
                                                StringRef StrTable,
-                                               bool IsDynamic) {
+                                               bool IsDynamic) const {
   StringRef SymbolName = unwrapOrError(Symbol->getName(StrTable));
----------------
echristo wrote:
> Can you do this separately?
Same as above.

================
Comment at: tools/llvm-readobj/StreamWriter.h:62
@@ -61,3 +61,3 @@
 raw_ostream &operator<<(raw_ostream &OS, const HexNumber& Value);
-const std::string to_hexString(uint64_t Value, bool UpperCase = true);
+const std::string to_hexString(uint64_t Value, bool UpperCase = false);
 const std::string to_string(uint64_t Value);
----------------
echristo wrote:
> Can you make this change separately?
Will do.


Repository:
  rL LLVM

http://reviews.llvm.org/D17523