More cleanup

3 files changed, 120 insertions, 112 deletions

diff --git a/src/components/utils/CMakeLists.txt b/src/components/utils/CMakeLists.txt
index 198094d50a..837a237c6c 100644
--- a/src/components/utils/CMakeLists.txt
+++ b/src/components/utils/CMakeLists.txt
@@ -113,10 +113,8 @@ if(${CMAKE_SYSTEM_NAME} MATCHES "Linux")
 endif()
 
 add_library("Utils" ${SOURCES})
-# Add boost system
+# Add boost libraries to link
 list(APPEND LIBRARIES -lboost_system -lboost_filesystem)
-
-
 target_link_libraries("Utils" ${LIBRARIES})
 
 if(ENABLE_LOG)
diff --git a/src/components/utils/include/utils/file_system.h b/src/components/utils/include/utils/file_system.h
index 53614d64b2..6d442110d4 100644
--- a/src/components/utils/include/utils/file_system.h
+++ b/src/components/utils/include/utils/file_system.h
@@ -33,17 +33,18 @@
 #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>
 
 namespace file_system {
 
 /**
- * @brief Get available disc space of directory. Returns 0 if the directory does not exist.
+ * @brief Get available disc space of directory. Returns 0 if the directory does
+ * not exist.
  *
  * @param path to directory
  * @return free disc space.
@@ -51,14 +52,15 @@ 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
@@ -80,76 +82,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 Doe 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 +163,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 +232,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);
 
 /**
@@ -257,25 +266,23 @@ bool CreateFile(const std::string& path);
 uint64_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 0c44659280..ed5866806d 100644
--- a/src/components/utils/src/file_system.cc
+++ b/src/components/utils/src/file_system.cc
@@ -224,10 +224,12 @@ void file_system::remove_directory_content(const std::string& directory_name) {
   }
 
   // According to Boost's documentation, removing shouldn't invalidate the
-  // iterator
+  // 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) {
     boost::uintmax_t num_removed = fs::remove_all(dirent, ec);
-    if (num_removed == 0 || ec) {
+    if (ec) {
       LOG4CXX_WARN_WITH_ERRNO(
           logger_, "Unable to remove file: " << dirent.path().string());
     }
@@ -358,9 +360,10 @@ bool file_system::CreateFile(const std::string& path) {
   }
 }
 
-// NOTE: this seems to have two flaws
+// NOTE: this seems to be buggy
 // It returns only the ns _component_ of modify time, not the whole thing
-// It relies on C11 / C++17 behaviour
+// see https://github.com/smartdevicelink/sdl_core/pull/1436/commits for a
+// potential fix / further explanation
 uint64_t file_system::GetFileModificationTime(const std::string& path) {
   struct stat info;
   if (0 != stat(path.c_str(), &info)) {