[PATCH] D56588: [llvm-objdump] - Change the output for --all-headers.

[James Henderson via Phabricator via llvm-commits]

jhenderson accepted this revision.
jhenderson added a comment.
This revision is now accepted and ready to land.

LGTM.



================
Comment at: tools/llvm-objdump/llvm-objdump.cpp:2304
   outs() << "start address: "
-         << "0x" << format(Fmt.data(), Address)
-         << "\n";
+         << "0x" << format(Fmt.data(), Address) << "\n\n";
 }
----------------
grimar wrote:
> jhenderson wrote:
> > Won't the extra '\n' result in a weird extra blank line when printing just the file headers?
> > 
> > I also noticed that there was a lot of inconsistency between the different outputs in this area. What I'd like to see is each different dump to append a blank line, only if there is more to come.
> > 
> > I don't feel too strongly about it though, as long as we are consistent.
> With that change it becomes consistent with what is done for COFF:
> https://github.com/llvm-mirror/llvm/blob/master/tools/llvm-objdump/llvm-objdump.cpp#L2437
> 
> It prints an additional empty line after "start address" but does not seem it look too bad perhaps:
> (I mean the last extra line, the first one looks a bit wierd to me, but it was already there).
> 
> > umb at umb-virtual-machine:~/LLVM$ /home/umb/LLVM/build/bin/llvm-objdump ooo --file-headers
> > 
> > ooo:	file format ELF64-x86-64
> > 
> > architecture: x86_64
> > start address: 0x0000000000201000
> > 
> > umb at umb-virtual-machine:~/LLVM$ /home/umb/LLVM/build/bin/llvm-objdump ooo --section-headers
> 
> I am also noticed a few inconsistencies (and also code style issues), but I think it is better to fix/refactor in a separate patch(es),
> all at once perhaps.
> 
Okay, sounds reasonable.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56588/new/

https://reviews.llvm.org/D56588