diff options
author | JackLivio <jack@livio.io> | 2018-08-31 10:46:35 -0400 |
---|---|---|
committer | GitHub <noreply@github.com> | 2018-08-31 10:46:35 -0400 |
commit | 9badd669439df0acec69aa2a658f457cacd9f34d (patch) | |
tree | 8f39aa79a74aa9c4be9feb9cb0d58d4229f4e148 | |
parent | ddc79ce414adcd966bdab61b814ce8a422e780f8 (diff) | |
parent | e42740ac6a534e8e18b73e449dece58d651f2ffb (diff) | |
download | sdl_core-9badd669439df0acec69aa2a658f457cacd9f34d.tar.gz |
Merge pull request #2226 from smartdevicelink/feature/boost_filesystem_implementation
Feature/boost filesystem implementation
11 files changed, 283 insertions, 293 deletions
diff --git a/src/3rd_party/CMakeLists.txt b/src/3rd_party/CMakeLists.txt index f207a38c3d..bdb96de417 100644 --- a/src/3rd_party/CMakeLists.txt +++ b/src/3rd_party/CMakeLists.txt @@ -30,8 +30,8 @@ include("./set_3rd_party_paths.cmake") -set(3RD_PARTY_SOURCE_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}) -set(3RD_PARTY_BINARY_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}) +set(3RD_PARTY_SOURCE_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}) +set(3RD_PARTY_BINARY_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}) set (install-3rd_party_logger_var "") @@ -209,7 +209,7 @@ else() ) endif() -find_package(Boost 1.66.0 COMPONENTS system thread date_time) +find_package(Boost 1.66.0 COMPONENTS system thread date_time filesystem) set(BOOST_LIB_SOURCE_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/boost_src) set(BOOST_LIBS_DIRECTORY ${3RD_PARTY_INSTALL_PREFIX}/lib) SET_PROPERTY(GLOBAL PROPERTY GLOBAL_BOOST_LIBS ${BOOST_LIBS_DIRECTORY}) @@ -226,9 +226,9 @@ if (NOT ${Boost_FOUND}) URL https://dl.bintray.com/boostorg/release/1.66.0/source/boost_1_66_0.tar.gz DOWNLOAD_DIR ${BOOST_LIB_SOURCE_DIRECTORY} SOURCE_DIR ${BOOST_LIB_SOURCE_DIRECTORY} - CONFIGURE_COMMAND ./bootstrap.sh --with-libraries=system,thread,date_time --prefix=${3RD_PARTY_INSTALL_PREFIX} + CONFIGURE_COMMAND ./bootstrap.sh --with-libraries=system,thread,date_time,filesystem --prefix=${3RD_PARTY_INSTALL_PREFIX} BUILD_COMMAND ./b2 - INSTALL_COMMAND ${BOOST_INSTALL_COMMAND} --with-system --with-thread --with-date_time --prefix=${3RD_PARTY_INSTALL_PREFIX} > boost_install.log + INSTALL_COMMAND ${BOOST_INSTALL_COMMAND} --with-system --with-thread --with-date_time --with-filesystem --prefix=${3RD_PARTY_INSTALL_PREFIX} > boost_install.log INSTALL_DIR ${3RD_PARTY_INSTALL_PREFIX} BUILD_IN_SOURCE true ) @@ -245,4 +245,3 @@ add_custom_target(install-3rd_party DEPENDS Boost WORKING_DIRECTORY ${3RD_PARTY_BINARY_DIRECTORY} ) - diff --git a/src/components/application_manager/include/application_manager/application_manager_impl.h b/src/components/application_manager/include/application_manager/application_manager_impl.h index b169839f23..d5f9f9b7f8 100644 --- a/src/components/application_manager/include/application_manager/application_manager_impl.h +++ b/src/components/application_manager/include/application_manager/application_manager_impl.h @@ -803,7 +803,7 @@ class ApplicationManagerImpl mobile_apis::Result::eType SaveBinary(const std::vector<uint8_t>& binary_data, const std::string& file_path, const std::string& file_name, - const int64_t offset) OVERRIDE; + const uint64_t offset) OVERRIDE; /** * @brief Get available app space diff --git a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/include/sdl_rpc_plugin/commands/mobile/put_file_request.h b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/include/sdl_rpc_plugin/commands/mobile/put_file_request.h index f82af197b9..2783280f87 100644 --- a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/include/sdl_rpc_plugin/commands/mobile/put_file_request.h +++ b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/include/sdl_rpc_plugin/commands/mobile/put_file_request.h @@ -70,7 +70,7 @@ class PutFileRequest : public app_mngr::commands::CommandRequestImpl { virtual void Run(); private: - int64_t offset_; + uint64_t offset_; std::string sync_file_name_; int64_t length_; mobile_apis::FileType::eType file_type_; 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 e6332d85c8..e79fb315ce 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 @@ -203,7 +203,12 @@ void SetAppIconRequest::CopyToIconStorage( void SetAppIconRequest::RemoveOldestIcons(const std::string& storage, const uint32_t icons_amount) const { const std::vector<std::string> icons_list = file_system::ListFiles(storage); - std::map<uint64_t, std::string> icon_modification_time; + auto compareTime = [](const time_t t1, const time_t t2) + -> bool { return difftime(t1, t2) > 0; }; + std::map<time_t, + std::string, + std::function<bool(const time_t, const time_t)> > + icon_modification_time(compareTime); std::vector<std::string>::const_iterator it = icons_list.begin(); for (; it != icons_list.end(); ++it) { const std::string file_name = *it; @@ -211,7 +216,7 @@ void SetAppIconRequest::RemoveOldestIcons(const std::string& storage, if (!file_system::FileExists(file_path)) { continue; } - const uint64_t time = file_system::GetFileModificationTime(file_path); + const time_t time = file_system::GetFileModificationTime(file_path); icon_modification_time[time] = file_name; } diff --git a/src/components/application_manager/src/application_manager_impl.cc b/src/components/application_manager/src/application_manager_impl.cc index 3f877895ef..69040997fe 100644 --- a/src/components/application_manager/src/application_manager_impl.cc +++ b/src/components/application_manager/src/application_manager_impl.cc @@ -2997,7 +2997,7 @@ mobile_apis::Result::eType ApplicationManagerImpl::SaveBinary( const std::vector<uint8_t>& binary_data, const std::string& file_path, const std::string& file_name, - const int64_t offset) { + const uint64_t offset) { LOG4CXX_DEBUG(logger_, "SaveBinaryWithOffset binary_size = " << binary_data.size() << " offset = " << offset); @@ -3008,7 +3008,7 @@ mobile_apis::Result::eType ApplicationManagerImpl::SaveBinary( } const std::string full_file_path = file_path + "/" + file_name; - const int64_t file_size = file_system::FileSize(full_file_path); + const uint64_t file_size = file_system::FileSize(full_file_path); std::ofstream* file_stream; if (offset != 0) { if (file_size != offset) { diff --git a/src/components/include/application_manager/application_manager.h b/src/components/include/application_manager/application_manager.h index bce2878f69..9b482b9e62 100644 --- a/src/components/include/application_manager/application_manager.h +++ b/src/components/include/application_manager/application_manager.h @@ -472,7 +472,7 @@ class ApplicationManager { const std::vector<uint8_t>& binary_data, const std::string& file_path, const std::string& file_name, - const int64_t offset) = 0; + const uint64_t offset) = 0; /* * @brief Sets SDL access to all mobile apps * diff --git a/src/components/include/test/application_manager/mock_application_manager.h b/src/components/include/test/application_manager/mock_application_manager.h index c4405b0ffd..e6e39bce58 100644 --- a/src/components/include/test/application_manager/mock_application_manager.h +++ b/src/components/include/test/application_manager/mock_application_manager.h @@ -186,7 +186,7 @@ class MockApplicationManager : public application_manager::ApplicationManager { mobile_apis::Result::eType(const std::vector<uint8_t>& binary_data, const std::string& file_path, const std::string& file_name, - const int64_t offset)); + const uint64_t offset)); MOCK_METHOD1(SetAllAppsAllowed, void(const bool allowed)); MOCK_METHOD1( set_driver_distraction_state, diff --git a/src/components/utils/CMakeLists.txt b/src/components/utils/CMakeLists.txt index 9db1312844..98da4cf2ab 100644 --- a/src/components/utils/CMakeLists.txt +++ b/src/components/utils/CMakeLists.txt @@ -119,12 +119,13 @@ GET_PROPERTY(BOOST_LIBS_DIRECTORY GLOBAL PROPERTY GLOBAL_BOOST_LIBS) list(APPEND LIBRARIES boost_system -L${BOOST_LIBS_DIRECTORY}) list(APPEND LIBRARIES boost_thread -L${BOOST_LIBS_DIRECTORY}) list(APPEND LIBRARIES boost_date_time -L${BOOST_LIBS_DIRECTORY}) +list(APPEND LIBRARIES boost_filesystem -L${BOOST_LIBS_DIRECTORY}) target_link_libraries("Utils" ${LIBRARIES}) add_dependencies("Utils" Boost) if(ENABLE_LOG) - add_dependencies("Utils" install-3rd_party_logger) + add_dependencies("Utils" install-3rd_party_logger Boost) endif() if(BUILD_TESTS) diff --git a/src/components/utils/include/utils/file_system.h b/src/components/utils/include/utils/file_system.h index 22c200934f..e26fef34bb 100644 --- a/src/components/utils/include/utils/file_system.h +++ b/src/components/utils/include/utils/file_system.h @@ -33,17 +33,19 @@ #ifndef SRC_COMPONENTS_UTILS_INCLUDE_UTILS_FILE_SYSTEM_H_ #define SRC_COMPONENTS_UTILS_INCLUDE_UTILS_FILE_SYSTEM_H_ -#include <string.h> #include <stdint.h> +#include <string.h> +#include <fstream> +#include <iostream> #include <string> #include <vector> -#include <iostream> -#include <fstream> +#include <time.h> namespace file_system { /** - * @brief Get available disc space. + * @brief Get available disc space of directory. Returns 0 if the directory does + * not exist. * * @param path to directory * @return free disc space. @@ -51,22 +53,23 @@ namespace file_system { uint64_t GetAvailableDiskSpace(const std::string& path); /* - * @brief Get size of current directory + * @brief Get size of given directory * * @param path to directory + * @return size of directory, return 0 if dir not exist */ size_t DirectorySize(const std::string& path); /* - * @brief Get size of current file + * @brief Get size of given file * * @param path to file * @return size of file, return 0 if file not exist */ -int64_t FileSize(const std::string& path); +uint64_t FileSize(const std::string& path); /** - * @brief Creates directory + * @brief Creates directory with owner_all permissions * @param name path to directory * @return path to created directory. */ @@ -80,76 +83,75 @@ std::string CreateDirectory(const std::string& name); bool CreateDirectoryRecursively(const std::string& path); /** - * @brief Checks the file to see whether the file is a directory - * @param name path to file - * @return returns true if file is directory. - */ + * @brief Checks the file to see whether the file is a directory + * @param name path to file + * @return returns true if file is directory. + */ bool IsDirectory(const std::string& name); /** - * @brief Is directory exist - * @param name path to directory - * @return returns true if directory is exists. - */ + * @brief Does directory exist + * @param name path to directory + * @return returns true if the file exists and is a directory. + */ bool DirectoryExists(const std::string& name); /** - * @brief Is file exist - * @param name path to file - * @return returns true if file is exists. - */ + * @brief Does file exist + * @param name path to file + * @return returns true if the file exists. + */ bool FileExists(const std::string& name); /** - * @brief Writes to file - * - * @remark - create file if it doesn't exist - * @param name path to file - * @param data data to write - * @return returns true if the operation is successfully. - */ + * @brief Writes to file + * + * @remark - create file if it doesn't exist + * @param name path to file + * @param data data to write + * @return returns true if the operation is successful. + */ bool Write(const std::string& file_name, const std::vector<uint8_t>& data, std::ios_base::openmode mode = std::ios_base::out); /** - * @brief Opens file stream for writing - * @param file_name path to file to write data to - * @return returns pointer to opened stream in case of success; - * otherwise returns NULL - */ + * @brief Opens file stream for writing + * @param file_name path to file to write data to + * @return returns pointer to opened stream in case of success; + * otherwise returns NULL + */ std::ofstream* Open(const std::string& file_name, std::ios_base::openmode mode = std::ios_base::out); /** - * @brief Writes to file stream - * @param file_stream file stream to be written to - * @param data data to be written to file - * @param data_size size of data to be written to file - * @return returns true if the operation is successfully. - */ + * @brief Writes to file stream + * @param file_stream file stream to be written to + * @param data data to be written to file + * @param data_size size of data to be written to file + * @return returns true if the operation is successful. + */ bool Write(std::ofstream* const file_stream, const uint8_t* data, uint32_t data_size); /** - * @brief Closes file stream - * @param file_stream file stream to be closed - */ + * @brief Closes file stream + * @param file_stream file stream to be closed + */ void Close(std::ofstream* file_stream); /** - * @brief Returns current working directory path - * If filename begins with "/", return unchanged filename - * @param name file name - * @return returns full file path. - */ + * @brief Returns current working directory path + * @return returns full file path. + */ std::string CurrentWorkingDirectory(); /** - * @brief Allows to obtaine absolute path for certain path. - * @param path the file name for which absolute path have to be calculated. - * @return absolute path for certain path. + * @brief Convert a path to its absolute form. + * @param path the file name to convert. + * @return corresponding absolute path for a valid path, otherwise an empty + * string. */ std::string GetAbsolutePath(const std::string& path); @@ -162,55 +164,63 @@ std::string GetAbsolutePath(const std::string& path); bool IsFileNameValid(const std::string& file_name); /** - * @brief Removes file - * - * @param name path to file - * @return returns true if the file is successfully deleted. - */ + * @brief Removes file + * + * @param name path to file + * @return returns true if the file is successfully deleted. + */ bool DeleteFile(const std::string& name); /** + * @brief Removes contents of directory but not directory itself + * + * @param directory_name path to directory. + */ +void remove_directory_content(const std::string& directory_name); + +/** * @brief Removes directory. * - * @param name path to directory. + * @param directory_name path to directory. * @param is_recursively true if you need delete directory recursively, - *otherwise false. + * otherwise false. A non-empty directory with is_recursively == false will + * return false. * @return returns true if the directory is successfully deleted. */ bool RemoveDirectory(const std::string& directory_name, bool is_recursively = true); /** - * @brief Check access rights - * - * @param name path to file. - * @param how Read/write attribute. - * @return returns true if file has the given mode. - */ + * @brief Check access rights + * + * @param name path to file. + * @param how Read/write attribute. + * @return returns true if file has the given mode. + */ bool IsAccessible(const std::string& name, int32_t how); /** - * @brief Check access rights for writing - * - * @param name path to file or folder - * @return returns true if has access rights. - */ + * @brief Check access rights for writing + * + * @param name path to file or folder + * @return returns true if has access rights. + */ bool IsWritingAllowed(const std::string& name); /** - * @brief Check access rights for reading - * - * @param name path to file. - * @return returns true if file has access rights. - */ + * @brief Check access rights for reading + * + * @param name path to file. + * @return returns true if file has access rights. + */ bool IsReadingAllowed(const std::string& name); /** - * @brief Lists all files in given directory - * - * @param name path to directory. - * @return returns list of files. - */ + * @brief Lists all files in given directory + * + * @param name path to directory. + * @return returns list of files. + */ std::vector<std::string> ListFiles(const std::string& directory_name); /** @@ -223,30 +233,30 @@ bool WriteBinaryFile(const std::string& name, const std::vector<uint8_t>& contents); /** - * @brief Reads from file - * - * @param name path to file - * @param result read data - * @return returns true if the operation is successfully. - */ + * @brief Reads from file + * + * @param name path to file + * @param result read data + * @return returns true if the operation is successfully. + */ bool ReadBinaryFile(const std::string& name, std::vector<uint8_t>& result); bool ReadFile(const std::string& name, std::string& result); /** - * @brief Convert special symbols in system path to percent-encoded - * - * @param name path to file - * @return returns converted path. -*/ + * @brief Convert special symbols in system path to percent-encoded + * + * @param name path to file + * @return returns converted path. + */ const std::string ConvertPathForURL(const std::string& path); /** - * @brief Create empty file - * - * @param name path to file - * @return if result success return true -*/ + * @brief Create empty file + * + * @param name path to file + * @return if result success return true + */ bool CreateFile(const std::string& path); /** @@ -254,28 +264,26 @@ bool CreateFile(const std::string& path); * @param path Path to file * @return Modification time in nanoseconds */ -uint64_t GetFileModificationTime(const std::string& path); +time_t GetFileModificationTime(const std::string& path); /** - * @brief Copy file from source to destination - * - * @param src Source file path - * @param dst Destination file path - * @return if result success return true -*/ + * @brief Copy file from source to destination + * + * @param src Source file path + * @param dst Destination file path + * @return if result success return true + */ bool CopyFile(const std::string& src, const std::string& dst); /** - * @brief Move file from source to destination - * - * @param src Source file path - * @param dst Destination file path - * @return if result success return true -*/ + * @brief Move file from source to destination + * + * @param src Source file path + * @param dst Destination file path + * @return if result success return true + */ bool MoveFile(const std::string& src, const std::string& dst); -void remove_directory_content(const std::string& directory_name); - } // namespace file_system #endif // SRC_COMPONENTS_UTILS_INCLUDE_UTILS_FILE_SYSTEM_H_ diff --git a/src/components/utils/src/file_system.cc b/src/components/utils/src/file_system.cc index 62a090550d..f98aeda056 100644 --- a/src/components/utils/src/file_system.cc +++ b/src/components/utils/src/file_system.cc @@ -33,122 +33,114 @@ #include "utils/file_system.h" #include "utils/logger.h" -#include <sys/statvfs.h> #include <sys/stat.h> +#include <sys/statvfs.h> #include <sys/types.h> #include <sstream> #include <dirent.h> #include <unistd.h> // TODO(VS): lint error: Streams are highly discouraged. -#include <fstream> +#include <algorithm> +#include <boost/filesystem.hpp> #include <cstddef> #include <cstdio> -#include <algorithm> +#include <fstream> CREATE_LOGGERPTR_GLOBAL(logger_, "Utils") +// Easier reference +namespace fs = boost::filesystem; +using boost::system::error_code; + uint64_t file_system::GetAvailableDiskSpace(const std::string& path) { - struct statvfs fsInfo = {0}; - if (statvfs(path.c_str(), &fsInfo) == 0) { - return fsInfo.f_bsize * fsInfo.f_bfree; - } else { + error_code ec; + fs::space_info si = fs::space(path, ec); + + if (ec) { + // If something went wrong, assume no free space return 0; + } else { + return si.free; } } -int64_t file_system::FileSize(const std::string& path) { - if (file_system::FileExists(path)) { - struct stat file_info = {0}; - if (0 != stat(path.c_str(), &file_info)) { - LOG4CXX_WARN_WITH_ERRNO(logger_, "Could not get file size: " << path); - } else { - return file_info.st_size; - } +uint64_t file_system::FileSize(const std::string& path) { + error_code ec; + // Boost returns sizes as unsigned + uint64_t fsize = (uint64_t)fs::file_size(path, ec); + + if (ec) { + LOG4CXX_WARN_WITH_ERRNO(logger_, "Could not get file size: " << path); + return 0; } - return 0; + return fsize; } size_t file_system::DirectorySize(const std::string& path) { - size_t size = 0; - DIR* directory = NULL; - - struct dirent* result = NULL; - struct stat file_info = {0}; - directory = opendir(path.c_str()); - if (NULL != directory) { - result = readdir(directory); - for (; NULL != result; result = readdir(directory)) { - if (0 == strcmp(result->d_name, "..") || - 0 == strcmp(result->d_name, ".")) { - continue; - } - std::string full_element_path = path + "/" + result->d_name; - if (file_system::IsDirectory(full_element_path)) { - size += DirectorySize(full_element_path); - } else { - stat(full_element_path.c_str(), &file_info); - size += file_info.st_size; - } + 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) { + 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; } + iter++; // next entry } - closedir(directory); - return size; + return dir_size; } +// NOTE that boost makes 0777 permissions by default std::string file_system::CreateDirectory(const std::string& name) { - if (!DirectoryExists(name)) { - if (0 != mkdir(name.c_str(), S_IRWXU)) { - LOG4CXX_WARN_WITH_ERRNO(logger_, "Unable to create directory: " << name); - } + error_code ec; + bool success = fs::create_directory(name, ec); + if (!success || ec) { + LOG4CXX_WARN_WITH_ERRNO(logger_, "Unable to create directory: " << name); + } else { + // Set 0700 permissions to maintain previous API + fs::permissions(name, fs::perms::owner_all, ec); } - return name; } bool file_system::CreateDirectoryRecursively(const std::string& path) { - size_t pos = 0; - bool ret_val = true; - - while (ret_val == true && pos <= path.length()) { - pos = path.find('/', pos + 1); - if (!DirectoryExists(path.substr(0, pos))) { - if (0 != mkdir(path.substr(0, pos).c_str(), S_IRWXU)) { - ret_val = false; - } - } - } + error_code ec; + // Create directory and all parents + fs::create_directories(path, ec); - return ret_val; -} + if (ec) { + LOG4CXX_WARN_WITH_ERRNO(logger_, + "Unable to create directory recursively: " + << path << " reason: " << ec.message()); -bool file_system::IsDirectory(const std::string& name) { - struct stat status = {0}; - - if (-1 == stat(name.c_str(), &status)) { return false; } - - return S_ISDIR(status.st_mode); + // return true if we made something or if it already existed + return true; } +bool file_system::IsDirectory(const std::string& name) { + error_code ec; + return fs::is_directory(name, ec); +} +// NOTE this may be a duplicate of IsDirectory since it already checks +// existence bool file_system::DirectoryExists(const std::string& name) { - struct stat status = {0}; - - if (-1 == stat(name.c_str(), &status) || !S_ISDIR(status.st_mode)) { - return false; - } - - return true; + return FileExists(name) && IsDirectory(name); } bool file_system::FileExists(const std::string& name) { - struct stat status = {0}; - - if (-1 == stat(name.c_str(), &status)) { - return false; - } - return true; + error_code ec; + return fs::exists(name, ec); } bool file_system::Write(const std::string& file_name, @@ -197,76 +189,75 @@ void file_system::Close(std::ofstream* file_stream) { } std::string file_system::CurrentWorkingDirectory() { - const size_t filename_max_length = 1024; - char path[filename_max_length]; - if (0 == getcwd(path, filename_max_length)) { + error_code ec; + fs::path currpath = fs::current_path(ec); + if (ec) { LOG4CXX_WARN(logger_, "Could not get CWD"); } - return std::string(path); + return currpath.string(); } std::string file_system::GetAbsolutePath(const std::string& path) { - char abs_path[PATH_MAX]; - if (NULL == realpath(path.c_str(), abs_path)) { - return std::string(); + error_code ec; + fs::path absolute = fs::canonical(path, ec); + if (ec) { + return std::string(); // invalid path } - - return std::string(abs_path); + return absolute.string(); } bool file_system::IsFileNameValid(const std::string& file_name) { 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) { if (FileExists(name) && IsAccessible(name, W_OK)) { - return !remove(name.c_str()); + error_code ec; + bool success = fs::remove(name.c_str(), ec); + return success && !ec; } return false; } void file_system::remove_directory_content(const std::string& directory_name) { - DIR* directory = NULL; - struct dirent* result = NULL; - - directory = opendir(directory_name.c_str()); - - if (NULL != directory) { - result = readdir(directory); - - for (; NULL != result; result = readdir(directory)) { - if (0 == strcmp(result->d_name, "..") || - 0 == strcmp(result->d_name, ".")) { - continue; - } - - std::string full_element_path = directory_name + "/" + result->d_name; - - if (file_system::IsDirectory(full_element_path)) { - remove_directory_content(full_element_path); - rmdir(full_element_path.c_str()); - } else { - if (0 != remove(full_element_path.c_str())) { - LOG4CXX_WARN_WITH_ERRNO( - logger_, "Unable to remove file: " << full_element_path); - } - } - } + error_code ec; + fs::directory_iterator dir_iter(directory_name, ec); + + if (ec) { + LOG4CXX_WARN_WITH_ERRNO(logger_, + "Unable to empty directory: " << directory_name); } - closedir(directory); + // 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); + if (ec) { + LOG4CXX_WARN_WITH_ERRNO( + logger_, "Unable to remove file: " << dirent.path().string()); + } + } } bool file_system::RemoveDirectory(const std::string& directory_name, bool is_recursively) { - if (DirectoryExists(directory_name) && IsAccessible(directory_name, W_OK)) { - if (is_recursively) { - remove_directory_content(directory_name); - } - - return !rmdir(directory_name.c_str()); + // Make sure the directory exists + if (!DirectoryExists(directory_name) && IsAccessible(directory_name, W_OK)) { + return false; } - return false; + error_code ec; + bool success; + // If recursive, just force full remove + if (is_recursively) { + success = (fs::remove_all(directory_name, ec) != 0); + } else { + // Otherwise try to remove + success = fs::remove(directory_name, ec); + } + return success && !ec; } bool file_system::IsAccessible(const std::string& name, int32_t how) { @@ -283,30 +274,20 @@ bool file_system::IsReadingAllowed(const std::string& name) { 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; - if (!DirectoryExists(directory_name)) { + + // In case the directory doesn't exist / can't be read, second check may be + // redundant + if (ec || !DirectoryExists(directory_name)) { return listFiles; } - DIR* directory = NULL; - struct dirent* result = NULL; - - directory = opendir(directory_name.c_str()); - if (NULL != directory) { - result = readdir(directory); - - for (; NULL != result; result = readdir(directory)) { - if (0 == strcmp(result->d_name, "..") || - 0 == strcmp(result->d_name, ".")) { - continue; - } - - listFiles.push_back(std::string(result->d_name)); - } - - closedir(directory); + for (auto& dirent : dir_iter) { + listFiles.push_back(dirent.path().filename().string()); } - return listFiles; } @@ -379,47 +360,42 @@ bool file_system::CreateFile(const std::string& path) { } } -uint64_t file_system::GetFileModificationTime(const std::string& path) { - struct stat info; - if (0 != stat(path.c_str(), &info)) { - LOG4CXX_WARN_WITH_ERRNO(logger_, "Could not get file mod time: " << path); +time_t file_system::GetFileModificationTime(const std::string& path) { + error_code ec; + std::time_t time = fs::last_write_time(path, ec); + if (ec) { + return 0; } - return static_cast<uint64_t>(info.st_mtim.tv_nsec); + return time; } bool file_system::CopyFile(const std::string& src, const std::string& dst) { if (!FileExists(src) || FileExists(dst) || !CreateFile(dst)) { return false; } - std::vector<uint8_t> data; - if (!ReadBinaryFile(src, data) || !WriteBinaryFile(dst, data)) { - DeleteFile(dst); + error_code ec; + fs::copy_file(src, dst, ec); + if (ec) { + // something failed return false; } return true; } bool file_system::MoveFile(const std::string& src, const std::string& dst) { + error_code ec; + if (std::rename(src.c_str(), dst.c_str()) == 0) { return true; - } else { - // In case of src and dst on different file systems std::rename returns - // an error (at least on QNX). - // Seems, streams are not recommended for use, so have - // to find another way to do this. - std::ifstream s_src(src, std::ios::binary); - if (!s_src.good()) { - return false; - } - std::ofstream s_dst(dst, std::ios::binary); - if (!s_dst.good()) { - return false; - } - s_dst << s_src.rdbuf(); - s_dst.close(); - s_src.close(); - DeleteFile(src); - return true; } - return false; + // In case of src and dst on different file systems std::rename returns + // an error (at least on QNX). + // Instead, copy the file over and delete the old one + bool success = CopyFile(src, dst); + if (!success) { + return false; + } + DeleteFile(src); + + return true; } diff --git a/src/components/utils/test/file_system_test.cc b/src/components/utils/test/file_system_test.cc index 95469766f8..eb35773fc9 100644 --- a/src/components/utils/test/file_system_test.cc +++ b/src/components/utils/test/file_system_test.cc @@ -990,13 +990,13 @@ TEST(FileSystemTest, TEST(FileSystemTest, WriteFileGetSize) { ASSERT_FALSE(FileExists("./test file")); EXPECT_TRUE(CreateFile("./test file")); - EXPECT_EQ(0, FileSize("./test file")); + EXPECT_EQ(0u, FileSize("./test file")); unsigned char tmp[] = {'t', 'e', 's', 't'}; std::vector<unsigned char> data(tmp, tmp + 4); EXPECT_TRUE(Write("./test file", data)); - EXPECT_NE(0, FileSize("./test file")); + EXPECT_NE(0u, FileSize("./test file")); EXPECT_TRUE(DeleteFile("./test file")); EXPECT_FALSE(FileExists("./test file")); @@ -1022,13 +1022,14 @@ TEST(FileSystemTest, GetFileModificationTime) { EXPECT_TRUE(CreateFile("./test file")); - uint64_t modif_time = GetFileModificationTime("./test file"); - EXPECT_LE(0ul, modif_time); + time_t modif_time = GetFileModificationTime("./test file"); + EXPECT_LE(0ul, static_cast<unsigned long>(modif_time)); std::vector<uint8_t> data(1, 1); EXPECT_TRUE(WriteBinaryFile("./test file", data)); - EXPECT_LE(0ul, GetFileModificationTime("./test file")); + EXPECT_LE(0ul, + static_cast<unsigned long>(GetFileModificationTime("./test file"))); EXPECT_LE(modif_time, GetFileModificationTime("./test file")); EXPECT_TRUE(DeleteFile("./test file")); |