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

[Jordan Rupprecht via Phabricator via llvm-commits]

rupprecht added inline comments.


================
Comment at: test/tools/llvm-readobj/readelf-s-alias.test:4-5
+RUN:   | FileCheck %s -check-prefix SEC
+RUN: llvm-readobj --sections %p/Inputs/trivial.obj.elf-i386 \
+RUN:   | FileCheck %s -check-prefix SEC
+
----------------
jhenderson wrote:
> Since -s should be an alias for --sections, maybe it's worth piping the -s output to a file and then comparing the output against --sections output.
> 
> Similar comment below for --symbols.
> 
> I assume we have sufficient separate testing for --symbols and --sections, so we can rely on those to show that we dump sections and symbols output correctly with the long-form switches. If we don't, we should add them.
> Since -s should be an alias for --sections, maybe it's worth piping the -s output to a file and then comparing the output against --sections output.
> 
> Similar comment below for --symbols.
Sorry, I don't understand this comment. This test is already running through FileCheck to make sure that -s and --sections both look like section output (for readobj) and similar for --symbols. Piping to a file and then comparing both files seems like it should pass, but is not really a targeted test -- the error would be "files differ" instead of "'Sections [' not found".

> 
> I assume we have sufficient separate testing for --symbols and --sections, so we can rely on those to show that we dump sections and symbols output correctly with the long-form switches. If we don't, we should add them.
The tests use short symbols by default. I flipped it so by default we use --symbols and --sections, with just a quick test that the aliases work.


Repository:
  rL LLVM

https://reviews.llvm.org/D54124