From 18028f9d431dba8bb538c989968f359734cb17e3 Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere Date: Tue, 23 Oct 2018 00:32:22 +0000 Subject: [PATCH] [dsymutil] Improve error reporting when we cannot create output file. Before this patch we were returning an empty string in case we couldn't create the output file. Now we return an expected string so we can return and print the proper issue. We now return errors instead of bools and defer printing to the call site. llvm-svn: 344983 --- llvm/tools/dsymutil/dsymutil.cpp | 73 +++++++++++++++++++++++----------------- 1 file changed, 42 insertions(+), 31 deletions(-) diff --git a/llvm/tools/dsymutil/dsymutil.cpp b/llvm/tools/dsymutil/dsymutil.cpp index c0e6d50..5fe4067 100644 --- a/llvm/tools/dsymutil/dsymutil.cpp +++ b/llvm/tools/dsymutil/dsymutil.cpp @@ -7,9 +7,8 @@ // //===----------------------------------------------------------------------===// // -// This program is a utility that aims to be a dropin replacement for -// Darwin's dsymutil. -// +// This program is a utility that aims to be a dropin replacement for Darwin's +// dsymutil. //===----------------------------------------------------------------------===// #include "dsymutil.h" @@ -165,20 +164,18 @@ static opt desc("Embed warnings in the linked DWARF debug info."), cat(DsymCategory)); -static bool createPlistFile(llvm::StringRef Bin, llvm::StringRef BundleRoot) { +static Error createPlistFile(llvm::StringRef Bin, llvm::StringRef BundleRoot) { if (NoOutput) - return true; + return Error::success(); // Create plist file to write to. llvm::SmallString<128> InfoPlist(BundleRoot); llvm::sys::path::append(InfoPlist, "Contents/Info.plist"); std::error_code EC; llvm::raw_fd_ostream PL(InfoPlist, EC, llvm::sys::fs::F_Text); - if (EC) { - WithColor::error() << "cannot create plist file " << InfoPlist << ": " - << EC.message() << '\n'; - return false; - } + if (EC) + return make_error( + "cannot create Plist: " + toString(errorCodeToError(EC)), EC); CFBundleInfo BI = getBundleInfo(Bin); @@ -230,22 +227,21 @@ static bool createPlistFile(llvm::StringRef Bin, llvm::StringRef BundleRoot) { << "\n"; PL.close(); - return true; + return Error::success(); } -static bool createBundleDir(llvm::StringRef BundleBase) { +static Error createBundleDir(llvm::StringRef BundleBase) { if (NoOutput) - return true; + return Error::success(); llvm::SmallString<128> Bundle(BundleBase); llvm::sys::path::append(Bundle, "Contents", "Resources", "DWARF"); - if (std::error_code EC = create_directories(Bundle.str(), true, - llvm::sys::fs::perms::all_all)) { - WithColor::error() << "cannot create directory " << Bundle << ": " - << EC.message() << "\n"; - return false; - } - return true; + if (std::error_code EC = + create_directories(Bundle.str(), true, llvm::sys::fs::perms::all_all)) + return make_error( + "cannot create bundle: " + toString(errorCodeToError(EC)), EC); + + return Error::success(); } static bool verify(llvm::StringRef OutputFile, llvm::StringRef Arch) { @@ -257,7 +253,7 @@ static bool verify(llvm::StringRef OutputFile, llvm::StringRef Arch) { Expected> BinOrErr = createBinary(OutputFile); if (!BinOrErr) { - errs() << OutputFile << ": " << toString(BinOrErr.takeError()); + WithColor::error() << OutputFile << ": " << toString(BinOrErr.takeError()); return false; } @@ -276,7 +272,7 @@ static bool verify(llvm::StringRef OutputFile, llvm::StringRef Arch) { return false; } -static std::string getOutputFileName(llvm::StringRef InputFile) { +static Expected getOutputFileName(llvm::StringRef InputFile) { // When updating, do in place replacement. if (OutputFileOpt.empty() && Update) return InputFile; @@ -305,8 +301,10 @@ static std::string getOutputFileName(llvm::StringRef InputFile) { llvm::SmallString<128> BundleDir(OutputFileOpt); if (BundleDir.empty()) BundleDir = DwarfFile + ".dSYM"; - if (!createBundleDir(BundleDir) || !createPlistFile(DwarfFile, BundleDir)) - return ""; + if (auto E = createBundleDir(BundleDir)) + return std::move(E); + if (auto E = createPlistFile(DwarfFile, BundleDir)) + return std::move(E); llvm::sys::path::append(BundleDir, "Contents", "Resources", "DWARF", llvm::sys::path::filename(DwarfFile)); @@ -521,13 +519,20 @@ int main(int argc, char **argv) { // Using a std::shared_ptr rather than std::unique_ptr because move-only // types don't work with std::bind in the ThreadPool implementation. std::shared_ptr OS; - std::string OutputFile = getOutputFileName(InputFile); + + Expected OutputFileOrErr = getOutputFileName(InputFile); + if (!OutputFileOrErr) { + WithColor::error() << toString(OutputFileOrErr.takeError()); + return 1; + } + + std::string OutputFile = *OutputFileOrErr; if (NeedsTempFiles) { TempFiles.emplace_back(Map->getTriple().getArchName().str()); auto E = TempFiles.back().createTempFile(); if (E) { - errs() << toString(std::move(E)); + WithColor::error() << toString(std::move(E)); return 1; } @@ -540,7 +545,7 @@ int main(int argc, char **argv) { OS = std::make_shared(NoOutput ? "-" : OutputFile, EC, sys::fs::F_None); if (EC) { - errs() << OutputFile << ": " << EC.message(); + WithColor::error() << OutputFile << ": " << EC.message(); return 1; } } @@ -567,10 +572,16 @@ int main(int argc, char **argv) { if (!AllOK) return 1; - if (NeedsTempFiles && - !MachOUtils::generateUniversalBinary( - TempFiles, getOutputFileName(InputFile), *OptionsOrErr, SDKPath)) - return 1; + if (NeedsTempFiles) { + Expected OutputFileOrErr = getOutputFileName(InputFile); + if (!OutputFileOrErr) { + WithColor::error() << toString(OutputFileOrErr.takeError()); + return 1; + } + if (!MachOUtils::generateUniversalBinary(TempFiles, *OutputFileOrErr, + *OptionsOrErr, SDKPath)) + return 1; + } } return 0; -- 2.7.4