From: Joel Rosdahl <joel@rosdahl.net>
Date: Sat, 8 Feb 2020 22:19:18 +0000 (+0100)
Subject: Improve the failed() and fatal() mechanisms
X-Git-Tag: v4.0~640
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=2adbab620f5905a08920f096fc40981c22f2a92f;p=thirdparty%2Fccache.git

Improve the failed() and fatal() mechanisms

The failure() and fatal() functions now exit by throwing exceptions that
are caught by the top level functions. This makes it possible to “throw
Failure” in functions that don’t have access to orig_args (or the future
context object).

While at it, renamed top-level functions to better reflect their
purpose.
---

diff --git a/dev.mk.in b/dev.mk.in
index 7e726ad43..8032083ef 100644
--- a/dev.mk.in
+++ b/dev.mk.in
@@ -36,7 +36,6 @@ built_dist_files = $(generated_sources) $(generated_docs)
 
 non_third_party_headers_without_cpp = \
     src/Checksum.hpp \
-    src/Error.hpp \
     src/File.hpp \
     src/FormatNonstdStringView.hpp \
     src/NonCopyable.hpp \
@@ -47,6 +46,7 @@ non_third_party_headers_without_cpp = \
     src/Stat.hpp \
     src/StdMakeUnique.hpp \
     src/ThreadPool.hpp \
+    src/exceptions.hpp \
     src/macroskip.hpp \
     src/system.hpp \
     unittest/framework.hpp \
diff --git a/src/AtomicFile.cpp b/src/AtomicFile.cpp
index 9dad3a392..3626f4517 100644
--- a/src/AtomicFile.cpp
+++ b/src/AtomicFile.cpp
@@ -18,7 +18,7 @@
 
 #include "AtomicFile.hpp"
 
-#include "Error.hpp"
+#include "exceptions.hpp"
 #include "Util.hpp"
 
 #include "third_party/fmt/core.h"
diff --git a/src/CacheEntryReader.cpp b/src/CacheEntryReader.cpp
index 28090780b..1772e1a88 100644
--- a/src/CacheEntryReader.cpp
+++ b/src/CacheEntryReader.cpp
@@ -19,7 +19,7 @@
 #include "CacheEntryReader.hpp"
 
 #include "Compressor.hpp"
-#include "Error.hpp"
+#include "exceptions.hpp"
 
 #include "third_party/fmt/core.h"
 
diff --git a/src/CacheFile.hpp b/src/CacheFile.hpp
index 0c98d337d..d684f6911 100644
--- a/src/CacheFile.hpp
+++ b/src/CacheFile.hpp
@@ -20,7 +20,7 @@
 
 #include "system.hpp"
 
-#include "Error.hpp"
+#include "exceptions.hpp"
 #include "Stat.hpp"
 
 #include "third_party/fmt/core.h"
diff --git a/src/Compression.cpp b/src/Compression.cpp
index 08d50e530..ecf93fc37 100644
--- a/src/Compression.cpp
+++ b/src/Compression.cpp
@@ -19,7 +19,7 @@
 #include "Compression.hpp"
 
 #include "Config.hpp"
-#include "Error.hpp"
+#include "exceptions.hpp"
 
 namespace Compression {
 
diff --git a/src/Config.cpp b/src/Config.cpp
index d786fdd16..bfaef8da5 100644
--- a/src/Config.cpp
+++ b/src/Config.cpp
@@ -19,7 +19,7 @@
 #include "Config.hpp"
 
 #include "AtomicFile.hpp"
-#include "Error.hpp"
+#include "exceptions.hpp"
 #include "Util.hpp"
 #include "ccache.hpp"
 
diff --git a/src/Error.hpp b/src/Error.hpp
deleted file mode 100644
index 0e155e30f..000000000
--- a/src/Error.hpp
+++ /dev/null
@@ -1,28 +0,0 @@
-// Copyright (C) 2019 Joel Rosdahl and other contributors
-//
-// See doc/AUTHORS.adoc for a complete list of contributors.
-//
-// This program is free software; you can redistribute it and/or modify it
-// under the terms of the GNU General Public License as published by the Free
-// Software Foundation; either version 3 of the License, or (at your option)
-// any later version.
-//
-// This program is distributed in the hope that it will be useful, but WITHOUT
-// ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
-// FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
-// more details.
-//
-// You should have received a copy of the GNU General Public License along with
-// this program; if not, write to the Free Software Foundation, Inc., 51
-// Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
-
-#pragma once
-
-#include "system.hpp"
-
-#include <stdexcept>
-
-class Error : public std::runtime_error
-{
-  using std::runtime_error::runtime_error;
-};
diff --git a/src/NullCompressor.cpp b/src/NullCompressor.cpp
index f54438df7..9757fd984 100644
--- a/src/NullCompressor.cpp
+++ b/src/NullCompressor.cpp
@@ -18,7 +18,7 @@
 
 #include "NullCompressor.hpp"
 
-#include "Error.hpp"
+#include "exceptions.hpp"
 
 NullCompressor::NullCompressor(FILE* stream) : m_stream(stream)
 {
diff --git a/src/NullDecompressor.cpp b/src/NullDecompressor.cpp
index 789de4fa0..090bdf5d1 100644
--- a/src/NullDecompressor.cpp
+++ b/src/NullDecompressor.cpp
@@ -18,7 +18,7 @@
 
 #include "NullDecompressor.hpp"
 
-#include "Error.hpp"
+#include "exceptions.hpp"
 
 NullDecompressor::NullDecompressor(FILE* stream) : m_stream(stream)
 {
diff --git a/src/Stat.hpp b/src/Stat.hpp
index 8f78a3d68..7eb025101 100644
--- a/src/Stat.hpp
+++ b/src/Stat.hpp
@@ -20,7 +20,7 @@
 
 #include "system.hpp"
 
-#include "Error.hpp"
+#include "exceptions.hpp"
 
 #include <string>
 
diff --git a/src/ZstdCompressor.cpp b/src/ZstdCompressor.cpp
index a5d16854b..beaa5896d 100644
--- a/src/ZstdCompressor.cpp
+++ b/src/ZstdCompressor.cpp
@@ -18,7 +18,7 @@
 
 #include "ZstdCompressor.hpp"
 
-#include "Error.hpp"
+#include "exceptions.hpp"
 #include "logging.hpp"
 
 const uint8_t k_default_zstd_compression_level = -1;
diff --git a/src/ZstdDecompressor.cpp b/src/ZstdDecompressor.cpp
index 9b8658602..75a8b1661 100644
--- a/src/ZstdDecompressor.cpp
+++ b/src/ZstdDecompressor.cpp
@@ -18,7 +18,7 @@
 
 #include "ZstdDecompressor.hpp"
 
-#include "Error.hpp"
+#include "exceptions.hpp"
 
 ZstdDecompressor::ZstdDecompressor(FILE* stream)
   : m_stream(stream),
diff --git a/src/ccache.cpp b/src/ccache.cpp
index b807e1cc6..79e60e117 100644
--- a/src/ccache.cpp
+++ b/src/ccache.cpp
@@ -20,7 +20,6 @@
 #include "ccache.hpp"
 
 #include "ArgsInfo.hpp"
-#include "Error.hpp"
 #include "FormatNonstdStringView.hpp"
 #include "ProgressBar.hpp"
 #include "ScopeGuard.hpp"
@@ -29,6 +28,7 @@
 #include "cleanup.hpp"
 #include "compopt.hpp"
 #include "compress.hpp"
+#include "exceptions.hpp"
 #include "execute.hpp"
 #include "exitfn.hpp"
 #include "hash.hpp"
@@ -186,22 +186,13 @@ add_prefix(struct args* args, const char* prefix_command)
   args_free(prefix);
 }
 
-static void failed(void) ATTR_NORETURN;
+static void failed(enum stats stat = STATS_NONE) ATTR_NORETURN;
 
 // Something went badly wrong - just execute the real compiler.
 static void
-failed(void)
+failed(enum stats stat)
 {
-  assert(orig_args);
-
-  args_strip(orig_args, "--ccache-");
-  add_prefix(orig_args, g_config.prefix_command().c_str());
-
-  cc_log("Failed; falling back to running the real compiler");
-  cc_log_argv("Executing ", orig_args->argv);
-  exitfn_call();
-  execv(orig_args->argv[0], orig_args->argv);
-  fatal("execv of %s failed: %s", orig_args->argv[0], strerror(errno));
+  throw Failure(stat);
 }
 
 static const char*
@@ -3603,11 +3594,12 @@ configuration_printer(const std::string& key,
   fmt::print("({}) {} = {}\n", origin, key, value);
 }
 
-static void ccache(int argc, char* argv[]) ATTR_NORETURN;
+static void cache_compilation(int argc, char* argv[]) ATTR_NORETURN;
+static void do_cache_compilation(char* argv[]) ATTR_NORETURN;
 
-// The main ccache driver function.
+// The entry point when invoked to cache a compilation.
 static void
-ccache(int argc, char* argv[])
+cache_compilation(int argc, char* argv[])
 {
 #ifndef _WIN32
   set_up_signal_handlers();
@@ -3619,10 +3611,34 @@ ccache(int argc, char* argv[])
   orig_args = args_init(argc, argv);
 
   initialize();
+
   MTR_BEGIN("main", "find_compiler");
   find_compiler(argv);
   MTR_END("main", "find_compiler");
 
+  try {
+    do_cache_compilation(argv);
+  } catch (const Failure& e) {
+    if (e.stat() != STATS_NONE) {
+      stats_update(e.stat());
+    }
+
+    assert(orig_args);
+
+    args_strip(orig_args, "--ccache-");
+    add_prefix(orig_args, g_config.prefix_command().c_str());
+
+    cc_log("Failed; falling back to running the real compiler");
+    cc_log_argv("Executing ", orig_args->argv);
+    exitfn_call();
+    execv(orig_args->argv[0], orig_args->argv);
+    fatal("execv of %s failed: %s", orig_args->argv[0], strerror(errno));
+  }
+}
+
+static void
+do_cache_compilation(char* argv[])
+{
   MTR_BEGIN("main", "clean_up_internal_tempdir");
   if (g_config.temporary_dir().empty()) {
     clean_up_internal_tempdir();
@@ -3856,7 +3872,7 @@ ccache(int argc, char* argv[])
 
 // The main program when not doing a compile.
 static int
-ccache_main_options(int argc, char* argv[])
+handle_main_options(int argc, char* argv[])
 {
   enum longopts {
     DUMP_MANIFEST,
@@ -4061,13 +4077,13 @@ ccache_main(int argc, char* argv[])
       // If the first argument isn't an option, then assume we are being passed
       // a compiler name and options.
       if (argv[1][0] == '-') {
-        return ccache_main_options(argc, argv);
+        return handle_main_options(argc, argv);
       }
     }
 
-    ccache(argc, argv);
-  } catch (const Error& e) {
-    fmt::print("ccache: error: {}\n", e.what());
-    return 1;
+    cache_compilation(argc, argv);
+  } catch (const ErrorBase& e) {
+    fmt::print(stderr, "ccache: error: {}\n", e.what());
+    return EXIT_FAILURE;
   }
 }
diff --git a/src/exceptions.hpp b/src/exceptions.hpp
new file mode 100644
index 000000000..f9bb78b6f
--- /dev/null
+++ b/src/exceptions.hpp
@@ -0,0 +1,69 @@
+// Copyright (C) 2019-2020 Joel Rosdahl and other contributors
+//
+// See doc/AUTHORS.adoc for a complete list of contributors.
+//
+// This program is free software; you can redistribute it and/or modify it
+// under the terms of the GNU General Public License as published by the Free
+// Software Foundation; either version 3 of the License, or (at your option)
+// any later version.
+//
+// This program is distributed in the hope that it will be useful, but WITHOUT
+// ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+// FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+// more details.
+//
+// You should have received a copy of the GNU General Public License along with
+// this program; if not, write to the Free Software Foundation, Inc., 51
+// Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+
+#pragma once
+
+#include "system.hpp"
+
+#include "stats.hpp"
+
+#include <stdexcept>
+
+// Don't throw or catch ErrorBase directly, use a subclass.
+class ErrorBase : public std::runtime_error
+{
+  using std::runtime_error::runtime_error;
+};
+
+// Throw an Error to indicate a potentially non-fatal error that may be caught
+// and handled by callers. An uncaught Error that reaches the top level will be
+// treated similar to FatalError.
+class Error : public ErrorBase
+{
+  using ErrorBase::ErrorBase;
+};
+
+// Throw a FatalError to make ccache print the error message to stderr and exit
+// with a non-zero exit code.
+class FatalError : public ErrorBase
+{
+  using ErrorBase::ErrorBase;
+};
+
+// Throw a Failure to make ccache fall back to running the real compiler. Also
+// updates statistics counter `stat` if it's not STATS_NONE.
+class Failure : public std::exception
+{
+public:
+  Failure(enum stats stat = STATS_NONE);
+
+  enum stats stat() const;
+
+private:
+  enum stats m_stat;
+};
+
+inline Failure::Failure(enum stats stat) : m_stat(stat)
+{
+}
+
+inline enum stats
+Failure::stat() const
+{
+  return m_stat;
+}
diff --git a/src/legacy_util.cpp b/src/legacy_util.cpp
index 8aee912c3..728aeb0da 100644
--- a/src/legacy_util.cpp
+++ b/src/legacy_util.cpp
@@ -19,8 +19,8 @@
 
 #include "legacy_util.hpp"
 
-#include "Error.hpp"
 #include "Util.hpp"
+#include "exceptions.hpp"
 #include "logging.hpp"
 
 #include "third_party/fmt/core.h"
@@ -85,10 +85,7 @@ fatal(const char* format, ...)
   vsnprintf(msg, sizeof(msg), format, ap);
   va_end(ap);
 
-  cc_log("FATAL: %s", msg);
-  fprintf(stderr, "ccache: error: %s\n", msg);
-
-  x_exit(1);
+  throw FatalError(msg);
 }
 
 // Copy all data from fd_in to fd_out.
diff --git a/src/legacy_util.hpp b/src/legacy_util.hpp
index d18266ecb..55f8902c2 100644
--- a/src/legacy_util.hpp
+++ b/src/legacy_util.hpp
@@ -22,6 +22,8 @@
 
 #include <string>
 
+void fatal(const char* format, ...) ATTR_FORMAT(printf, 1, 2) ATTR_NORETURN;
+
 bool copy_fd(int fd_in, int fd_out);
 bool clone_file(const char* src, const char* dest, bool via_tmp_file);
 bool copy_file(const char* src, const char* dest, bool via_tmp_file);
diff --git a/src/logging.cpp b/src/logging.cpp
index 44f5a6d3c..cfcd81980 100644
--- a/src/logging.cpp
+++ b/src/logging.cpp
@@ -20,6 +20,7 @@
 #include "logging.hpp"
 
 #include "Config.hpp"
+#include "exceptions.hpp"
 #include "execute.hpp"
 
 #ifdef HAVE_SYSLOG_H
diff --git a/src/logging.hpp b/src/logging.hpp
index c0ca0d87a..e80be653d 100644
--- a/src/logging.hpp
+++ b/src/logging.hpp
@@ -26,4 +26,3 @@ void cc_log(const char* format, ...) ATTR_FORMAT(printf, 1, 2);
 void cc_bulklog(const char* format, ...) ATTR_FORMAT(printf, 1, 2);
 void cc_log_argv(const char* prefix, char** argv);
 void cc_dump_debug_log_buffer(const char* path);
-void fatal(const char* format, ...) ATTR_FORMAT(printf, 1, 2) ATTR_NORETURN;
diff --git a/src/result.cpp b/src/result.cpp
index d60903632..971567f09 100644
--- a/src/result.cpp
+++ b/src/result.cpp
@@ -22,7 +22,7 @@
 #include "CacheEntryReader.hpp"
 #include "CacheEntryWriter.hpp"
 #include "Config.hpp"
-#include "Error.hpp"
+#include "exceptions.hpp"
 #include "File.hpp"
 #include "Stat.hpp"
 #include "Util.hpp"
diff --git a/unittest/test_Config.cpp b/unittest/test_Config.cpp
index c3e485fd9..5496f7faa 100644
--- a/unittest/test_Config.cpp
+++ b/unittest/test_Config.cpp
@@ -17,7 +17,7 @@
 // Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
 
 #include "../src/Config.hpp"
-#include "../src/Error.hpp"
+#include "../src/exceptions.hpp"
 #include "../src/Util.hpp"
 #include "../src/ccache.hpp"