diff options
author | Jacob Keeler <jacob.keeler@livioradio.com> | 2020-01-16 12:10:00 -0500 |
---|---|---|
committer | GitHub <noreply@github.com> | 2020-01-16 12:10:00 -0500 |
commit | e9709e670196427035466f07786fb0d22d484017 (patch) | |
tree | 6fe42aefaa16aa0d63503bd88c03e57e59497af7 | |
parent | 86d541bc9d5bbe6e525caa858effd5bf76892582 (diff) | |
parent | 6eb1e5774dde538b3e1021b9320e000114b978b8 (diff) | |
download | sdl_core-e9709e670196427035466f07786fb0d22d484017.tar.gz |
Merge pull request #3145 from smartdevicelink/fix/SDL_crash_during_SetAppIconremote_atf_testing
Fix/sdl crash during set app icon
4 files changed, 262 insertions, 104 deletions
diff --git a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/include/sdl_rpc_plugin/commands/mobile/set_app_icon_request.h b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/include/sdl_rpc_plugin/commands/mobile/set_app_icon_request.h index 86ca2fb126..520f27d8b9 100644 --- a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/include/sdl_rpc_plugin/commands/mobile/set_app_icon_request.h +++ b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/include/sdl_rpc_plugin/commands/mobile/set_app_icon_request.h @@ -83,9 +83,11 @@ class SetAppIconRequest : public app_mngr::commands::CommandRequestImpl { private: /** * @brief Copies file to icon storage + * @param policy_app_id application policy app id * @param path_to_file Path to icon */ - void CopyToIconStorage(const std::string& path_to_file) const; + void CopyToIconStorage(const std::string& policy_app_id, + const std::string& path_to_file) const; /** * @brief Remove oldest icons @@ -108,6 +110,11 @@ class SetAppIconRequest : public app_mngr::commands::CommandRequestImpl { * @brief Checks, if icons saving to configured folder is enabled */ bool is_icons_saving_enabled_; + + /** + * @brief Contains full file path to icon + */ + std::string full_file_path_for_hmi_; }; } // namespace commands diff --git a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/set_app_icon_request.cc b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/set_app_icon_request.cc index b2363e870f..acf312d532 100644 --- a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/set_app_icon_request.cc +++ b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/set_app_icon_request.cc @@ -106,25 +106,22 @@ void SetAppIconRequest::Run() { msg_params[strings::sync_file_name] = smart_objects::SmartObject(smart_objects::SmartType_Map); - // Panasonic requres unchanged path value without encoded special characters - const std::string full_file_path_for_hmi = - file_system::ConvertPathForURL(full_file_path); + // For further use in on_event function + full_file_path_for_hmi_ = file_system::ConvertPathForURL(full_file_path); - msg_params[strings::sync_file_name][strings::value] = full_file_path_for_hmi; + msg_params[strings::sync_file_name][strings::value] = full_file_path_for_hmi_; // TODO(VS): research why is image_type hardcoded msg_params[strings::sync_file_name][strings::image_type] = static_cast<int32_t>(SetAppIconRequest::ImageType::DYNAMIC); - // for further use in on_event function - (*message_)[strings::msg_params][strings::sync_file_name] = - msg_params[strings::sync_file_name]; StartAwaitForInterface(HmiInterfaces::HMI_INTERFACE_UI); SendHMIRequest(hmi_apis::FunctionID::UI_SetAppIcon, &msg_params, true); } void SetAppIconRequest::CopyToIconStorage( - const std::string& path_to_file) const { + const std::string& policy_app_id, const std::string& path_to_file) const { + LOG4CXX_AUTO_TRACE(logger_); if (!(application_manager_.protocol_handler() .get_settings() .max_supported_protocol_version() >= @@ -146,6 +143,11 @@ void SetAppIconRequest::CopyToIconStorage( application_manager_.get_settings().app_icons_folder_max_size()); const uint64_t file_size = file_system::FileSize(path_to_file); + if (0 == file_size) { + LOG4CXX_ERROR(logger_, "Can't get the icon file size: " << path_to_file); + return; + } + if (storage_max_size < file_size) { LOG4CXX_ERROR(logger_, "Icon size (" << file_size @@ -159,6 +161,12 @@ void SetAppIconRequest::CopyToIconStorage( const uint64_t storage_size = static_cast<uint64_t>(file_system::DirectorySize(icon_storage)); + + if (0 == storage_size) { + LOG4CXX_ERROR(logger_, "Can't get the folder size: " << icon_storage); + return; + } + if (storage_max_size < (file_size + storage_size)) { const uint32_t icons_amount = application_manager_.get_settings().app_icons_amount_to_remove(); @@ -174,23 +182,15 @@ void SetAppIconRequest::CopyToIconStorage( RemoveOldestIcons(icon_storage, icons_amount); } } - ApplicationConstSharedPtr app = - application_manager_.application(connection_key()); - if (!app) { - LOG4CXX_ERROR( - logger_, - "Can't get application for connection key: " << connection_key()); - return; - } + const std::string icon_path = icon_storage + "/" + policy_app_id; - const std::string icon_path = icon_storage + "/" + app->policy_app_id(); if (!file_system::CreateFile(icon_path)) { LOG4CXX_ERROR(logger_, "Can't create icon: " << icon_path); return; } - if (!file_system::Write(icon_path, file_content)) { + if (!file_system::WriteBinaryFile(icon_path, file_content)) { LOG4CXX_ERROR(logger_, "Can't write icon: " << icon_path); return; } @@ -266,21 +266,19 @@ void SetAppIconRequest::on_event(const event_engine::Event& event) { ApplicationSharedPtr app = application_manager_.application(connection_key()); - if ((message_.use_count() == 0) || (app.use_count() == 0)) { - LOG4CXX_ERROR(logger_, "NULL pointer."); + if (!app) { + LOG4CXX_ERROR( + logger_, + "Can't get application for connection key: " << connection_key()); return; } - const std::string& path = - (*message_)[strings::msg_params][strings::sync_file_name] - [strings::value] - .asString(); - - if (is_icons_saving_enabled_) { - CopyToIconStorage(path); + if (is_icons_saving_enabled_ && !full_file_path_for_hmi_.empty()) { + const auto policy_app_id = app->policy_app_id(); + CopyToIconStorage(policy_app_id, full_file_path_for_hmi_); } - app->set_app_icon_path(path); + app->set_app_icon_path(full_file_path_for_hmi_); LOG4CXX_INFO(logger_, "Icon path was set to '" << app->app_icon_path() << "'"); diff --git a/src/components/utils/include/utils/file_system.h b/src/components/utils/include/utils/file_system.h index 9e0dc233af..f575da129f 100644 --- a/src/components/utils/include/utils/file_system.h +++ b/src/components/utils/include/utils/file_system.h @@ -70,10 +70,10 @@ uint64_t FileSize(const std::string& path); /** * @brief Creates directory with owner_all permissions - * @param path directory to create - * @return true if directory was created or already exist. + * @param name directory to create + * @return true if directory was created or already existed. */ -bool CreateDirectory(const std::string& path); +bool CreateDirectory(const std::string& name); /** * @brief Creates directory recursively @@ -188,7 +188,7 @@ void remove_directory_content(const std::string& directory_name); * @return returns true if the directory is successfully deleted. */ bool RemoveDirectory(const std::string& directory_name, - bool is_recursively = true); + const bool is_recursively = true); /** * @brief Check access rights @@ -197,7 +197,7 @@ bool RemoveDirectory(const std::string& directory_name, * @param how Read/write attribute. * @return returns true if file has the given mode. */ -bool IsAccessible(const std::string& name, int32_t how); +bool IsAccessible(const std::string& name, const int32_t how); /** * @brief Check access rights for writing diff --git a/src/components/utils/src/file_system.cc b/src/components/utils/src/file_system.cc index c2fb8a4eb4..015dc25a22 100644 --- a/src/components/utils/src/file_system.cc +++ b/src/components/utils/src/file_system.cc @@ -47,80 +47,116 @@ #include <cstdio> #include <fstream> -CREATE_LOGGERPTR_GLOBAL(logger_, "Utils") +CREATE_LOGGERPTR_GLOBAL(logger_, "Utils::FileSystem") // Easier reference namespace fs = boost::filesystem; using boost::system::error_code; uint64_t file_system::GetAvailableDiskSpace(const std::string& path) { + LOG4CXX_AUTO_TRACE(logger_); error_code ec; - fs::space_info si = fs::space(path, ec); + fs::space_info si = {0, 0, 0}; + si = fs::space(path, ec); if (ec) { // If something went wrong, assume no free space - return 0; - } else { - return si.free; + LOG4CXX_ERROR_WITH_ERRNO(logger_, + "Unable to get available disk space: '" + << path << "', reason: " << ec.message()); } + return si.free; } uint64_t file_system::FileSize(const std::string& path) { + LOG4CXX_AUTO_TRACE(logger_); error_code ec; // Boost returns sizes as unsigned - uint64_t fsize = (uint64_t)fs::file_size(path, ec); + const uint64_t fsize = static_cast<uint64_t>(fs::file_size(path, ec)); if (ec) { - LOG4CXX_WARN_WITH_ERRNO(logger_, "Could not get file size: " << path); + LOG4CXX_ERROR_WITH_ERRNO( + logger_, + "Unable to get file size: '" << path << "', reason: " << ec.message()); return 0; } return fsize; } size_t file_system::DirectorySize(const std::string& path) { + LOG4CXX_AUTO_TRACE(logger_); size_t dir_size = 0; error_code ec; // Recursively iterate through directory to accumulate file sizes fs::recursive_directory_iterator iter(path, ec); - // Directory does not exist if (ec) { + LOG4CXX_ERROR_WITH_ERRNO(logger_, + "Unable to get directory size: '" + << path << "', reason: " << ec.message()); return 0; } + // default constructor gives end iterator fs::recursive_directory_iterator end; - while (iter != end) { - size_t fsize = fs::file_size(iter->path(), ec); - // No error means we can add the file - if (!ec) { - dir_size += fsize; + while (end != iter) { + const bool is_directory = fs::is_directory(iter->path(), ec); + if (ec) { + LOG4CXX_WARN_WITH_ERRNO( + logger_, + "Failed check if '" << iter->path() + << "' is directory, reason: " << ec.message()); + } + + if (!is_directory && !ec) { + const size_t fsize = fs::file_size(iter->path(), ec); + if (ec) { + LOG4CXX_WARN_WITH_ERRNO(logger_, + "Failed to get file_size: '" + << path << "', reason: " << ec.message()); + } else { + // No error means we can add the file + dir_size += fsize; + LOG4CXX_DEBUG(logger_, "Adding: " << fsize << ", total: " << dir_size); + } + } + + // Increment the iterator to point to next entry in recursive iteration + iter.increment(ec); + if (ec) { + LOG4CXX_ERROR_WITH_ERRNO(logger_, + "Failed to increment iterator for path '" + << path << "', reason: " << ec.message()); + return dir_size; } - iter++; // next entry } + return dir_size; } // NOTE that boost makes 0777 permissions by default -bool file_system::CreateDirectory(const std::string& path) { +bool file_system::CreateDirectory(const std::string& name) { + LOG4CXX_AUTO_TRACE(logger_); error_code ec; - const bool success = fs::create_directory(path, ec); + const bool success = fs::create_directory(name, ec); if (!success || ec) { - LOG4CXX_WARN_WITH_ERRNO(logger_, "Unable to create directory: " << path); + LOG4CXX_WARN_WITH_ERRNO(logger_, "Unable to create directory: " << name); } else { // Set 0700 permissions to maintain previous API - fs::permissions(path, fs::perms::owner_all, ec); + fs::permissions(name, fs::perms::owner_all, ec); } return success; } bool file_system::CreateDirectoryRecursively(const std::string& path) { + LOG4CXX_AUTO_TRACE(logger_); error_code ec; // Create directory and all parents fs::create_directories(path, ec); if (ec) { - LOG4CXX_WARN_WITH_ERRNO(logger_, - "Unable to create directory recursively: " - << path << " reason: " << ec.message()); + LOG4CXX_ERROR_WITH_ERRNO(logger_, + "Unable to create directory recursively: '" + << path << "', reason: " << ec.message()); return false; } @@ -129,23 +165,44 @@ bool file_system::CreateDirectoryRecursively(const std::string& path) { } bool file_system::IsDirectory(const std::string& name) { + LOG4CXX_AUTO_TRACE(logger_); error_code ec; - return fs::is_directory(name, ec); + const bool is_directory = fs::is_directory(name, ec); + if (ec) { + LOG4CXX_ERROR_WITH_ERRNO(logger_, + "Unable to check if it is directory: " + << name << " reason: " << ec.message()); + } + return is_directory; } + // NOTE this may be a duplicate of IsDirectory since it already checks // existence bool file_system::DirectoryExists(const std::string& name) { - return FileExists(name) && IsDirectory(name); + LOG4CXX_AUTO_TRACE(logger_); + const bool exists = FileExists(name) && IsDirectory(name); + LOG4CXX_DEBUG( + logger_, + "Directory '" << name << "' " << (exists ? "exists" : "NOT exists")); + return exists; } bool file_system::FileExists(const std::string& name) { + LOG4CXX_AUTO_TRACE(logger_); error_code ec; - return fs::exists(name, ec); + const bool exists = fs::exists(name, ec); + if (ec) { + LOG4CXX_ERROR_WITH_ERRNO(logger_, + "Unable to check that file exists: " + << name << " reason: " << ec.message()); + } + return exists; } bool file_system::Write(const std::string& file_name, const std::vector<uint8_t>& data, std::ios_base::openmode mode) { + LOG4CXX_AUTO_TRACE(logger_); std::ofstream file(file_name.c_str(), std::ios_base::binary | mode); if (file.is_open()) { for (uint32_t i = 0; i < data.size(); ++i) { @@ -159,6 +216,7 @@ bool file_system::Write(const std::string& file_name, std::ofstream* file_system::Open(const std::string& file_name, std::ios_base::openmode mode) { + LOG4CXX_AUTO_TRACE(logger_); std::ofstream* file = new std::ofstream(); file->open(file_name.c_str(), std::ios_base::binary | mode); if (file->is_open()) { @@ -166,12 +224,13 @@ std::ofstream* file_system::Open(const std::string& file_name, } delete file; - return NULL; + return nullptr; } bool file_system::Write(std::ofstream* const file_stream, const uint8_t* data, uint32_t data_size) { + LOG4CXX_AUTO_TRACE(logger_); bool result = false; if (file_stream) { for (size_t i = 0; i < data_size; ++i) { @@ -183,69 +242,105 @@ bool file_system::Write(std::ofstream* const file_stream, } void file_system::Close(std::ofstream* file_stream) { + LOG4CXX_AUTO_TRACE(logger_); if (file_stream) { file_stream->close(); } } std::string file_system::CurrentWorkingDirectory() { + LOG4CXX_AUTO_TRACE(logger_); error_code ec; - fs::path currpath = fs::current_path(ec); + const fs::path currpath = fs::current_path(ec); if (ec) { - LOG4CXX_WARN(logger_, "Could not get CWD"); + LOG4CXX_WARN_WITH_ERRNO(logger_, + "Unable to get current working directory: '" + << currpath << "' reason: " << ec.message()); } return currpath.string(); } std::string file_system::GetAbsolutePath(const std::string& path) { + LOG4CXX_AUTO_TRACE(logger_); error_code ec; - fs::path absolute = fs::canonical(path, ec); + const fs::path absolute = fs::canonical(path, ec); if (ec) { + LOG4CXX_ERROR_WITH_ERRNO(logger_, + "Unable to get absolute path: '" + << path << "', reason: " << ec.message()); return std::string(); // invalid path } return absolute.string(); } bool file_system::IsFileNameValid(const std::string& file_name) { + LOG4CXX_AUTO_TRACE(logger_); return file_name.end() == std::find(file_name.begin(), file_name.end(), '/'); } // Does not remove if file is write-protected bool file_system::DeleteFile(const std::string& name) { + LOG4CXX_AUTO_TRACE(logger_); if (FileExists(name) && IsAccessible(name, W_OK)) { error_code ec; - bool success = fs::remove(name.c_str(), ec); + const bool success = fs::remove(name.c_str(), ec); + if (ec) { + LOG4CXX_ERROR_WITH_ERRNO(logger_, + "Unable to delete file: '" + << name << "', reason: " << ec.message() + << "success: " << success); + } + return success && !ec; } + LOG4CXX_WARN( + logger_, + "Unable to delete file either doesn't exist or is not accessible"); return false; } void file_system::remove_directory_content(const std::string& directory_name) { + LOG4CXX_AUTO_TRACE(logger_); + error_code ec; fs::directory_iterator dir_iter(directory_name, ec); - if (ec) { - LOG4CXX_WARN_WITH_ERRNO(logger_, - "Unable to empty directory: " << directory_name); + LOG4CXX_ERROR_WITH_ERRNO( + logger_, + "Unable to remove directory contents: " << directory_name + << " reason: " << ec.message()); } // According to Boost's documentation, removing shouldn't invalidate the // iterator, although it may cause the removed entry to appear again, // duplicating the warning message. See here: // https://www.boost.org/doc/libs/1_67_0/libs/filesystem/doc/reference.html#Class-directory_iterator - for (auto& dirent : dir_iter) { - fs::remove_all(dirent, ec); + const fs::directory_iterator end; + while (dir_iter != end) { + fs::remove_all(dir_iter->path(), ec); if (ec) { - LOG4CXX_WARN_WITH_ERRNO( - logger_, "Unable to remove file: " << dirent.path().string()); + LOG4CXX_ERROR_WITH_ERRNO( + logger_, + "Unable to remove file: " << dir_iter->path().string() << " reason " + << ec.message()); + } + dir_iter.increment(ec); + if (ec) { + LOG4CXX_ERROR_WITH_ERRNO( + logger_, "Unable to increment dir_iter: reason " << ec.message()); + break; } } } bool file_system::RemoveDirectory(const std::string& directory_name, - bool is_recursively) { + const bool is_recursively) { + LOG4CXX_AUTO_TRACE(logger_); // Make sure the directory exists if (!DirectoryExists(directory_name) && IsAccessible(directory_name, W_OK)) { + LOG4CXX_WARN( + logger_, + "Unable to remove directory either doesn't exist or is not accessible"); return false; } error_code ec; @@ -253,42 +348,67 @@ bool file_system::RemoveDirectory(const std::string& directory_name, // If recursive, just force full remove if (is_recursively) { success = (fs::remove_all(directory_name, ec) != 0); + if (ec) { + LOG4CXX_ERROR_WITH_ERRNO(logger_, + "Unable to remove all: '" << directory_name + << "', reason " + << ec.message()); + } } else { // Otherwise try to remove success = fs::remove(directory_name, ec); + if (ec) { + LOG4CXX_ERROR_WITH_ERRNO(logger_, + "Unable to remove: '" << directory_name + << "', reason " + << ec.message()); + } } return success && !ec; } -bool file_system::IsAccessible(const std::string& name, int32_t how) { +bool file_system::IsAccessible(const std::string& name, const int32_t how) { + LOG4CXX_AUTO_TRACE(logger_); return !access(name.c_str(), how); } bool file_system::IsWritingAllowed(const std::string& name) { + LOG4CXX_AUTO_TRACE(logger_); return IsAccessible(name, W_OK); } bool file_system::IsReadingAllowed(const std::string& name) { + LOG4CXX_AUTO_TRACE(logger_); return IsAccessible(name, R_OK); } std::vector<std::string> file_system::ListFiles( const std::string& directory_name) { - error_code ec; - - fs::directory_iterator dir_iter(directory_name, ec); - std::vector<std::string> listFiles; + LOG4CXX_AUTO_TRACE(logger_); - // In case the directory doesn't exist / can't be read, second check may be - // redundant - if (ec || !DirectoryExists(directory_name)) { - return listFiles; + error_code ec; + fs::directory_iterator iter(directory_name, ec), end; + if (ec) { + LOG4CXX_ERROR_WITH_ERRNO( + logger_, + "Unable to get directory_iterator: " << directory_name << " reason " + << ec.message()); + return std::vector<std::string>(); } - for (auto& dirent : dir_iter) { - listFiles.push_back(dirent.path().filename().string()); + std::vector<std::string> list_files; + while (end != iter) { + list_files.push_back(iter->path().filename().string()); + iter.increment(ec); + if (ec) { + LOG4CXX_ERROR_WITH_ERRNO(logger_, + "Failed to increment iterator for path '" + << directory_name + << "', reason: " << ec.message()); + return list_files; + } } - return listFiles; + return list_files; } bool file_system::WriteBinaryFile(const std::string& name, @@ -302,17 +422,22 @@ bool file_system::WriteBinaryFile(const std::string& name, bool file_system::ReadBinaryFile(const std::string& name, std::vector<uint8_t>& result) { + LOG4CXX_AUTO_TRACE(logger_); + LOG4CXX_DEBUG(logger_, "Filename: " << name); if (!FileExists(name) || !IsAccessible(name, R_OK)) { + LOG4CXX_ERROR(logger_, "Not able to read binary file: " << name); return false; } - std::ifstream file(name.c_str(), std::ios_base::binary); - std::ostringstream ss; - ss << file.rdbuf(); - const std::string s = ss.str(); + std::ifstream file(name.c_str(), std::ios::in | std::ios_base::binary); + if (!file.is_open()) { + return false; + } + + std::vector<uint8_t> content((std::istreambuf_iterator<char>(file)), + std::istreambuf_iterator<char>()); + result.swap(content); - result.resize(s.length()); - std::copy(s.begin(), s.end(), result.begin()); return true; } @@ -321,12 +446,16 @@ bool file_system::ReadBinaryFile(const std::string& name, uint32_t offset, uint32_t length) { if (!FileExists(name) || !IsAccessible(name, R_OK)) { + LOG4CXX_ERROR(logger_, "Not able to read binary file: " << name); return false; } std::ifstream file(name.c_str(), std::ios_base::binary); + if (!file.is_open()) { + return false; + } + file.ignore(offset); - std::ostringstream ss; std::string s; s.resize(length); file.read(&s[0], length); @@ -337,11 +466,16 @@ bool file_system::ReadBinaryFile(const std::string& name, } bool file_system::ReadFile(const std::string& name, std::string& result) { + LOG4CXX_AUTO_TRACE(logger_); if (!FileExists(name) || !IsAccessible(name, R_OK)) { + LOG4CXX_ERROR(logger_, "Not able to read file: " << name); return false; } - std::ifstream file(name.c_str()); + if (!file) { + LOG4CXX_ERROR(logger_, "Not able to open binary file: " << name); + return false; + } std::ostringstream ss; ss << file.rdbuf(); result = ss.str(); @@ -349,53 +483,66 @@ bool file_system::ReadFile(const std::string& name, std::string& result) { } const std::string file_system::ConvertPathForURL(const std::string& path) { - std::string::const_iterator it_path = path.begin(); - std::string::const_iterator it_path_end = path.end(); - + LOG4CXX_AUTO_TRACE(logger_); const std::string reserved_symbols = "!#$&'()*+,:;=?@[] "; size_t pos = std::string::npos; std::string converted_path; - for (; it_path != it_path_end; ++it_path) { - pos = reserved_symbols.find_first_of(*it_path); + for (const auto symbol : path) { + pos = reserved_symbols.find_first_of(symbol); if (pos != std::string::npos) { const size_t size = 100; char percent_value[size]; - snprintf(percent_value, size, "%%%x", *it_path); + snprintf(percent_value, size, "%%%x", symbol); converted_path += percent_value; } else { - converted_path += *it_path; + converted_path += symbol; } } return converted_path; } bool file_system::CreateFile(const std::string& path) { + LOG4CXX_AUTO_TRACE(logger_); + std::ofstream file(path); if (!(file.is_open())) { + LOG4CXX_WARN(logger_, "failed to create file: " << path); return false; - } else { - file.close(); - return true; } + file.close(); + return true; } time_t file_system::GetFileModificationTime(const std::string& path) { + LOG4CXX_AUTO_TRACE(logger_); + error_code ec; std::time_t time = fs::last_write_time(path, ec); if (ec) { + LOG4CXX_ERROR_WITH_ERRNO(logger_, + "Unable to get file modification time: " + << path << " reason " << ec.message()); + return 0; } return time; } bool file_system::CopyFile(const std::string& src, const std::string& dst) { + LOG4CXX_AUTO_TRACE(logger_); if (!FileExists(src) || FileExists(dst) || !CreateFile(dst)) { + LOG4CXX_WARN( + logger_, + "Failed to copy file from: '" << src << "', to: '" << dst << "'"); return false; } error_code ec; fs::copy_file(src, dst, ec); if (ec) { + LOG4CXX_ERROR_WITH_ERRNO( + logger_, + "Unable to copy file: '" << src << "', reason: " << ec.message()); // something failed return false; } @@ -403,7 +550,7 @@ bool file_system::CopyFile(const std::string& src, const std::string& dst) { } bool file_system::MoveFile(const std::string& src, const std::string& dst) { - error_code ec; + LOG4CXX_AUTO_TRACE(logger_); if (std::rename(src.c_str(), dst.c_str()) == 0) { return true; @@ -413,9 +560,15 @@ bool file_system::MoveFile(const std::string& src, const std::string& dst) { // Instead, copy the file over and delete the old one bool success = CopyFile(src, dst); if (!success) { + LOG4CXX_ERROR( + logger_, + "Failed to copy file from: '" << src << "', to: '" << dst << "'"); + return false; + } + success = DeleteFile(src); + if (!success) { + LOG4CXX_ERROR(logger_, "Failed to delete file '" << src << "'"); return false; } - DeleteFile(src); - return true; } |