[PATCH] D59946: [llvm-readobj] Improve error message for for --string-dump

[Jordan Rupprecht via Phabricator via llvm-commits]

rupprecht marked 2 inline comments as done.
rupprecht added a comment.

I just have a couple nits on top of what James said, but overall this is already an improvement. Thanks!



================
Comment at: llvm/test/tools/llvm-readobj/elf-malformed-pt-dynamic.test:24
 
-# CHECK: Error reading file: Invalid data was encountered while parsing the file.
+# CHECK: error: Invalid data was encountered while parsing the file
 
----------------
Whatever is throwing this error should probably be creating a FileError
(I'm fine if this part is in a separate patch tho)


================
Comment at: llvm/tools/llvm-readobj/llvm-readobj.cpp:373
+  WithColor::error(errs()) << Msg;
+  errs() << "\n";
   errs().flush();
----------------
This can go on the previous line, i.e. `<< Msg << "\n";`


================
Comment at: llvm/tools/llvm-readobj/llvm-readobj.cpp:374
+  errs() << "\n";
   errs().flush();
   exit(1);
----------------
... completely unrelated to this patch, but is this even necessary? I thought stderr was always unbuffered. There will never be anything to flush.


================
Comment at: llvm/tools/llvm-readobj/llvm-readobj.cpp:405-406
 
-static void reportError(StringRef Input, Error Err) {
-  if (Input == "-")
-    Input = "<stdin>";
-  std::string ErrMsg;
-  {
-    raw_string_ostream ErrStream(ErrMsg);
-    logAllUnhandledErrors(std::move(Err), ErrStream, Input + ": ");
-  }
-  reportError(ErrMsg);
+static void reportError(StringRef Input, std::error_code EC) {
+  reportError(Input, errorCodeToError(EC));
 }
----------------
jhenderson wrote:
> Nit: Could you move this above the reportError immediately above, so that it lines up well in diffs with the old version, please?
I'm guessing this is actually necessary in order to delegate to `reportError(StringRef, Error)`


Repository:
  rL LLVM

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

https://reviews.llvm.org/D59946