[PATCH] D36780: [llvm-dlltool] Don't crash if no def file is provided or it can't be opened

[Rui Ueyama via Phabricator via llvm-commits]

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

LGTM



================
Comment at: lib/ToolDrivers/llvm-dlltool/DlltoolDriver.cpp:67
+  if (std::error_code EC = MB.getError()) {
     llvm::errs() << "fail openFile: " << EC.message() << "\n";
+    return None;
----------------
`openFile` is a function name and shouldn't be reported as part of an error message. It should be something like `"cannot open file " << Path << ": " << EC.message() << "\n"`.

But this is not new code, so we don't have to address it in this patch.


================
Comment at: lib/ToolDrivers/llvm-dlltool/DlltoolDriver.cpp:120-128
+  Optional<MemoryBufferRef> MB;
+  if (auto *Arg = Args.getLastArg(OPT_d)) {
     MB = openFile(Arg->getValue());
+    if (!MB)
+      return 1;
+  } else {
+    llvm::errs() << "no definition file specified\n";
----------------
I feel this way is more readable.

  if (!Args.hasArg(OPT_d))
    llvm::errs() << "no definition file specified\n";
    return 1;
  }

  Optional<MemoryBufferRef> MB = openFile(Args.getLastArg(OPT_d)->getValue());
  if (!MB)
    return 1;



https://reviews.llvm.org/D36780