From e16dc041ec7dbcc83c64533ee4cf8759f6ae0be5 Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Mon, 28 Sep 2009 18:49:51 -0700 Subject: 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. --- cache.cxx | 34 ++++++++-------------------------- main.cxx | 10 ++-------- tapsets.cxx | 25 +++++-------------------- util.cxx | 30 +++++++++++++++++++----------- util.h | 3 ++- 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& tokens, -- cgit