[PATCH] D54124: [llvm-readelf] Make llvm-readelf more compatible with GNU readelf.

Jordan Rupprecht via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Nov 9 10:25:12 PST 2018


rupprecht added a comment.

Updated the description, thanks for pointing that out.



================
Comment at: tools/llvm-readobj/llvm-readobj.cpp:82
+                               cl::aliasopt(Sections), cl::NotHidden);
+  cl::alias SectionHeadersAlias("section-headers",
+                                cl::desc("Alias for --sections"),
----------------
MaskRay wrote:
> jhenderson wrote:
> > This should be the "main" option, in my opinion, with the other two as aliases. Also, the help text for it should be "Display all section headers." since it doesn't display section contents.
> What's your opinion on the plural form `-file-headers`? I think I'm lost why in readelf, `-file-header` is singular while others are plural. Usually there are not more than one program headers, section headers, etc.
My guess: most file formats only have one file header (e.g. ELF just has the ELF header), so singular makes sense, e.g. vs section headers where there is one section header per section.

Looking at `COFFDumper::printFileHeaders()` though, there can be multiple file headers (ImageFileHeader, ImageOptionalHeader, DOSHeader), so actually I'm inclined to leave the plural version as the main flag: the tool will print any number of file headers, even if you only work in ELF and so it only ever prints one.


Repository:
  rL LLVM

https://reviews.llvm.org/D54124





More information about the llvm-commits mailing list