[PATCH] D55619: [elfabi] Add option to manually specify file read format

James Henderson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Dec 19 02:09:16 PST 2018


jhenderson added a comment.

Happy for this to be committed once my comments are addressed.



================
Comment at: llvm/test/tools/llvm-elfabi/read-tbe-as-elf.test:15
+
+#CHECK:         Error: Encountered multiple errors:
+#CHECK-NEXT:    0 (BinaryRead): The file was not recognized as a valid object file
----------------
Nit: I think the style is to have a space between the # and CHECK.


================
Comment at: llvm/test/tools/llvm-elfabi/read-tbe-as-elf.test:17
+#CHECK-NEXT:    0 (BinaryRead): The file was not recognized as a valid object file
+#CHECK-NEXT:    All file readers failed to read `{{.*}}`. (unsupported/malformed file?)
----------------
Maybe worth checking the filename part of this in this error message.


================
Comment at: llvm/test/tools/llvm-elfabi/read-tbe-as-tbe.test:2
+# RUN: llvm-elfabi -TBE %s -emit-tbe=%t
+# RUN: cat %t | FileCheck %s
+
----------------
Just run FileCheck --input-file=%t, rather than using cat.

Not sure if this available here, but something worth considering adding if it isn't. A lot of tests put a file on stdout using '-' as the filename. Doing so would then allow you to run FileCheck on it without needing to call cat or use --input-file:
`# RUN: llvm-elfabi -TBE %s -emit-tbe=- | FileCheck %s`


================
Comment at: llvm/tools/llvm-elfabi/llvm-elfabi.cpp:40
+                          "TBE", "Read `input` as text-based ELF stub"),
+               clEnumValN(FileFormat::TBE,
+                          "tbe", "(alias for TBE)"),
----------------
I take it there's no way of making the input argument match-up case insensitive? I know that options can be case-insensitive if the command line library is used appropriately.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D55619





More information about the llvm-commits mailing list