Simplify copy_file calls

Every single copy_file call we had was converting strings to char*,
printing the same error message, and optionally printing the same
verbose string.  Let's canonicalize that.

* util.cxx (copy_file): Take string filenames, add a verbose flag, and
  consolidate the message printing.
* cache.cxx (add_to_cache): Pass strings and remove message printing.
  (get_from_cache): Ditto.
* main.cxx (main): Ditto.
* tapsets.cxx (tracepoint_builder::get_tracequery_module): Ditto.
  (dwarf_cast_expanding_visitor::filter_special_modules): Ditto.

5 files changed, 36 insertions, 66 deletions

diff --git a/cache.cxx b/cache.cxx
index 73dd59ce..d9a44ea0 100644
--- a/cache.cxx
+++ b/cache.cxx
@@ -48,29 +48,23 @@ static void clean_cache(systemtap_session& s);
 void
 add_to_cache(systemtap_session& s)
 {
+  bool verbose = s.verbose > 1;
+
   // PR10543: clean the cache *before* we try putting something new into it.
   // We don't want to risk having the brand new contents being erased again.
   clean_cache(s);
 
   string stapconf_src_path = s.tmpdir + "/" + s.stapconf_name;
-  if (s.verbose > 1)
-    clog << "Copying " << stapconf_src_path << " to " << s.stapconf_path << endl;
-  if (copy_file(stapconf_src_path.c_str(), s.stapconf_path.c_str()) != 0)
+  if (!copy_file(stapconf_src_path, s.stapconf_path, verbose))
     {
-      cerr << "Copy failed (\"" << stapconf_src_path << "\" to \""
-	   << s.stapconf_path << "\"): " << strerror(errno) << endl;
       s.use_cache = false;
       return;
     }
 
   string module_src_path = s.tmpdir + "/" + s.module_name + ".ko";
   STAP_PROBE2(stap, cache__add__module, module_src_path.c_str(), s.hash_path.c_str());
-  if (s.verbose > 1)
-    clog << "Copying " << module_src_path << " to " << s.hash_path << endl;
-  if (copy_file(module_src_path.c_str(), s.hash_path.c_str()) != 0)
+  if (!copy_file(module_src_path, s.hash_path, verbose))
     {
-      cerr << "Copy failed (\"" << module_src_path << "\" to \""
-	   << s.hash_path << "\"): " << strerror(errno) << endl;
       s.use_cache = false;
       return;
     }
@@ -81,14 +75,8 @@ add_to_cache(systemtap_session& s)
   c_dest_path += ".c";
 
   STAP_PROBE2(stap, cache__add__source, s.translated_source.c_str(), c_dest_path.c_str());
-  if (s.verbose > 1)
-    clog << "Copying " << s.translated_source << " to " << c_dest_path
-	 << endl;
-  if (copy_file(s.translated_source.c_str(), c_dest_path.c_str()) != 0)
+  if (!copy_file(s.translated_source, c_dest_path, verbose))
     {
-      if (s.verbose > 1)
-        cerr << "Copy failed (\"" << s.translated_source << "\" to \""
-             << c_dest_path << "\"): " << strerror(errno) << endl;
       // NB: this is not so severe as to prevent reuse of the .ko
       // already copied.
       //
@@ -118,10 +106,8 @@ get_from_cache(systemtap_session& s)
     }
 
   // Copy the stapconf header file to the destination
-  if (copy_file(s.stapconf_path.c_str(), stapconf_dest_path.c_str()) != 0)
+  if (!copy_file(s.stapconf_path, stapconf_dest_path))
     {
-      cerr << "Copy failed (\"" << s.stapconf_path << "\" to \""
-	   << stapconf_dest_path << "\"): " << strerror(errno) << endl;
       close(fd_stapconf);
       return false;
     }
@@ -152,10 +138,8 @@ get_from_cache(systemtap_session& s)
     }
 
   // Copy the cached C file to the destination
-  if (copy_file(c_src_path.c_str(), s.translated_source.c_str()) != 0)
+  if (!copy_file(c_src_path, s.translated_source))
     {
-      cerr << "Copy failed (\"" << c_src_path << "\" to \""
-	   << s.translated_source << "\"): " << strerror(errno) << endl;
       close(fd_module);
       close(fd_c);
       return false;
@@ -164,10 +148,8 @@ get_from_cache(systemtap_session& s)
   // Copy the cached module to the destination (if needed)
   if (s.last_pass != 3)
     {
-      if (copy_file(s.hash_path.c_str(), module_dest_path.c_str()) != 0)
+      if (!copy_file(s.hash_path, module_dest_path))
         {
-	  cerr << "Copy failed (\"" << s.hash_path << "\" to \""
-	       << module_dest_path << "\"): " << strerror(errno) << endl;
 	  unlink(c_src_path.c_str());
 	  close(fd_module);
 	  close(fd_c);
diff --git a/main.cxx b/main.cxx
index 8e3c9399..6987d94a 100644
--- a/main.cxx
+++ b/main.cxx
@@ -1212,19 +1212,13 @@ main (int argc, char * const argv [])
       // inaccessible for some reason.
       if (! s.use_cache && s.last_pass == 4)
         save_module = true;
-      
+
       // Copy module to the current directory.
       if (save_module && !pending_interrupts)
         {
 	  string module_src_path = s.tmpdir + "/" + s.module_name + ".ko";
 	  string module_dest_path = s.module_name + ".ko";
-
-	  if (s.verbose > 1)
-	    clog << "Copying " << module_src_path << " to "
-		 << module_dest_path << endl;
-	  if (copy_file(module_src_path.c_str(), module_dest_path.c_str()) != 0)
-	    cerr << "Copy failed (\"" << module_src_path << "\" to \""
-		 << module_dest_path << "\"): " << strerror(errno) << endl;
+	  copy_file(module_src_path, module_dest_path, s.verbose > 1);
 	}
     }
 
diff --git a/tapsets.cxx b/tapsets.cxx
index dfa5f302..abd46aee 100644
--- a/tapsets.cxx
+++ b/tapsets.cxx
@@ -2566,17 +2566,9 @@ void dwarf_cast_expanding_visitor::filter_special_modules(string& module)
       // no cached module, time to make it
       if (make_typequery(s, module) == 0)
         {
+          // try to save typequery in the cache
           if (s.use_cache)
-            {
-              // try to save typequery in the cache
-              if (s.verbose > 2)
-                clog << "Copying " << module
-                  << " to " << cached_module << endl;
-              if (copy_file(module.c_str(),
-                            cached_module.c_str()) != 0)
-                cerr << "Copy failed (\"" << module << "\" to \""
-                  << cached_module << "\"): " << strerror(errno) << endl;
-            }
+            copy_file(module, cached_module, s.verbose > 2);
         }
     }
 }
@@ -6121,17 +6113,10 @@ tracepoint_builder::get_tracequery_module(systemtap_session& s,
   if (rc != 0)
     tracequery_ko = "/dev/null";
 
+  // try to save tracequery in the cache
   if (s.use_cache)
-    {
-      // try to save tracequery in the cache
-      if (s.verbose > 2)
-        clog << "Copying " << tracequery_ko
-             << " to " << tracequery_path << endl;
-      if (copy_file(tracequery_ko.c_str(),
-                    tracequery_path.c_str()) != 0)
-        cerr << "Copy failed (\"" << tracequery_ko << "\" to \""
-             << tracequery_path << "\"): " << strerror(errno) << endl;
-    }
+    copy_file(tracequery_ko, tracequery_path, s.verbose > 2);
+
   return tracequery_ko;
 }
 
diff --git a/util.cxx b/util.cxx
index 057cc7ab..b81d37cb 100644
--- a/util.cxx
+++ b/util.cxx
@@ -78,8 +78,8 @@ file_exists (const string &path)
 
 // Copy a file.  The copy is done via a temporary file and atomic
 // rename.
-int
-copy_file(const char *src, const char *dest)
+bool
+copy_file(const string& src, const string& dest, bool verbose)
 {
   int fd1, fd2;
   char buf[10240];
@@ -88,10 +88,13 @@ copy_file(const char *src, const char *dest)
   char *tmp_name;
   mode_t mask;
 
+  if (verbose)
+    clog << "Copying " << src << " to " << dest << endl;
+
   // Open the src file.
-  fd1 = open(src, O_RDONLY);
+  fd1 = open(src.c_str(), O_RDONLY);
   if (fd1 == -1)
-    return -1;
+    goto error;
 
   // Open the temporary output file.
   tmp = dest + string(".XXXXXX");
@@ -100,7 +103,7 @@ copy_file(const char *src, const char *dest)
   if (fd2 == -1)
     {
       close(fd1);
-      return -1;
+      goto error;
     }
 
   // Copy the src file to the temporary output file.
@@ -111,7 +114,7 @@ copy_file(const char *src, const char *dest)
 	  close(fd2);
 	  close(fd1);
 	  unlink(tmp_name);
-	  return -1;
+          goto error;
 	}
     }
   close(fd1);
@@ -126,18 +129,23 @@ copy_file(const char *src, const char *dest)
   if (close(fd2) == -1)
     {
       unlink(tmp_name);
-      return -1;
+      goto error;
     }
 
   // Rename the temporary output file to the destination file.
-  unlink(dest);
-  if (rename(tmp_name, dest) == -1)
+  unlink(dest.c_str());
+  if (rename(tmp_name, dest.c_str()) == -1)
     {
       unlink(tmp_name);
-      return -1;
+      goto error;
     }
 
-  return 0;
+  return true;
+
+error:
+  cerr << "Copy failed (\"" << src << "\" to \"" << dest << "\"): "
+       << strerror(errno) << endl;
+  return false;
 }
 
 
diff --git a/util.h b/util.h
index 24845545..1252d559 100644
--- a/util.h
+++ b/util.h
@@ -8,7 +8,8 @@
 const char *get_home_directory(void);
 size_t get_file_size(const std::string &path);
 bool file_exists (const std::string &path);
-int copy_file(const char *src, const char *dest);
+bool copy_file(const std::string& src, const std::string& dest,
+               bool verbose=false);
 int create_dir(const char *dir);
 int remove_file_or_dir(const char *dir);
 void tokenize(const std::string& str, std::vector<std::string>& tokens,