From 42677e06962697e372847cc6acea48018ea662bb Mon Sep 17 00:00:00 2001 From: Scott K Logan Date: Tue, 27 Oct 2020 15:21:01 -0700 Subject: [PATCH 1/7] Add an API for directory iteration Signed-off-by: Scott K Logan --- include/rcutils/filesystem.h | 48 +++++++++ src/filesystem.c | 198 ++++++++++++++++++++++++----------- test/test_filesystem.cpp | 67 ++++++++++++ 3 files changed, 249 insertions(+), 64 deletions(-) diff --git a/include/rcutils/filesystem.h b/include/rcutils/filesystem.h index 730f878c..5321801b 100644 --- a/include/rcutils/filesystem.h +++ b/include/rcutils/filesystem.h @@ -243,6 +243,54 @@ RCUTILS_PUBLIC size_t rcutils_get_file_size(const char * file_path); +/// An iterator used for enumerating directory contents +typedef struct rcutils_dir_iter_t +{ + /// The name of the enumerated file or directory + const char * entry_name; + /// The allocator used internally by iteration functions + rcutils_allocator_t allocator; + /// The platform-specific iteration state + void * state; +} rcutils_dir_iter_t; + +/// Begin iterating over the contents of the specified directory. +/* + * This function is used to list the file and directories that are contained in + * a specified directory. The structure returned by it must be deallocated using + * ::rcutils_dir_iter_end when the iteration is completed. The name of the + * enumerated entry is stored in the `entry_name` member of the returned object, + * and the first entry is already populated upon completion of this function. To + * populate the entry with the name of the next entry, use the + * ::rcutils_dir_iter_next function. Note that the "." and ".." entries are + * typically among the entries enumerated. + * \param[in] directory_path The directory path to iterate over the contents of. + * \param[in] allocator Allocator used to create the returned structure. + * \return An iterator object used to continue iterating directory contents + * \return NULL if an error occurred + */ +RCUTILS_PUBLIC +rcutils_dir_iter_t * +rcutils_dir_iter_start(const char * directory_path, const rcutils_allocator_t allocator); + +/// Continue iterating over the contents of a directory. +/* + * \param[in] iter An iterator created by ::rcutils_dir_iter_start. + * \return `True` if another entry was found + * \return `False` if there are no more entries in the directory + */ +RCUTILS_PUBLIC +bool +rcutils_dir_iter_next(rcutils_dir_iter_t * iter); + +/// Finish iterating over the contents of a directory. +/* + * \param[in] iter An iterator created by ::rcutils_dir_iter_start. + */ +RCUTILS_PUBLIC +void +rcutils_dir_iter_end(rcutils_dir_iter_t * iter); + #ifdef __cplusplus } #endif diff --git a/src/filesystem.c b/src/filesystem.c index a17b0627..6f5f79a2 100644 --- a/src/filesystem.c +++ b/src/filesystem.c @@ -54,6 +54,16 @@ extern "C" # define RCUTILS_PATH_DELIMITER "/" #endif // _WIN32 +typedef struct rcutils_dir_iter_state_t +{ +#ifdef _WIN32 + HANDLE handle; + WIN32_FIND_DATA data; +#else + DIR * dir; +#endif +} rcutils_dir_iter_state_t; + bool rcutils_get_cwd(char * buffer, size_t max_length) { @@ -337,6 +347,7 @@ rcutils_calculate_directory_size_with_recursion( { dir_list_t * dir_list = NULL; rcutils_ret_t ret = RCUTILS_RET_OK; + rcutils_dir_iter_t * iter = NULL; if (NULL == directory_path) { RCUTILS_SAFE_FWRITE_TO_STDERR("directory_path is NULL !"); @@ -365,41 +376,28 @@ rcutils_calculate_directory_size_with_recursion( dir_list->path = rcutils_strdup(directory_path, allocator); if (NULL == dir_list->path) { RCUTILS_SAFE_FWRITE_TO_STDERR("Failed to duplicate directory path !\n"); - return RCUTILS_RET_BAD_ALLOC; + ret = RCUTILS_RET_BAD_ALLOC; + goto fail; } *size = 0; -#ifdef _WIN32 - HANDLE handle = INVALID_HANDLE_VALUE; - char * dir_path = NULL; - do { - dir_path = rcutils_join_path(dir_list->path, "*", allocator); - if (NULL == dir_path) { - RCUTILS_SAFE_FWRITE_TO_STDERR("Failed to duplicate directory path !\n"); - ret = RCUTILS_RET_BAD_ALLOC; - goto fail; - } - - WIN32_FIND_DATA data; - handle = FindFirstFile(dir_path, &data); - if (INVALID_HANDLE_VALUE == handle) { - RCUTILS_SAFE_FWRITE_TO_STDERR_WITH_FORMAT_STRING( - "Can't open directory %s. Error code: %lu\n", dir_list->path, GetLastError()); + iter = rcutils_dir_iter_start(dir_list->path, allocator); + if (NULL == iter) { ret = RCUTILS_RET_ERROR; goto fail; } do { - ret = check_and_calculate_size(data.cFileName, size, max_depth, dir_list, allocator); + ret = check_and_calculate_size(iter->entry_name, size, max_depth, dir_list, allocator); if (RCUTILS_RET_OK != ret) { goto fail; } - } while (FindNextFile(handle, &data)); + } while (rcutils_dir_iter_next(iter)); - FindClose(handle); - allocator.deallocate(dir_path, allocator.state); + rcutils_dir_iter_end(iter); + iter = NULL; remove_first_dir_from_list(&dir_list, allocator); } while (dir_list); @@ -407,66 +405,138 @@ rcutils_calculate_directory_size_with_recursion( return ret; fail: - if (NULL != dir_path) { - allocator.deallocate(dir_path, allocator.state); + rcutils_dir_iter_end(iter); + free_dir_list(dir_list, allocator); + return ret; +} + +size_t +rcutils_get_file_size(const char * file_path) +{ + if (!rcutils_is_file(file_path)) { + RCUTILS_SAFE_FWRITE_TO_STDERR_WITH_FORMAT_STRING( + "Path is not a file: %s\n", file_path); + return 0; } - if (INVALID_HANDLE_VALUE != handle) { - FindClose(handle); + struct stat stat_buffer; + int rc = stat(file_path, &stat_buffer); + return rc == 0 ? (size_t)(stat_buffer.st_size) : 0; +} + +rcutils_dir_iter_t * +rcutils_dir_iter_start(const char * directory_path, const rcutils_allocator_t allocator) +{ + RCUTILS_CHECK_ARGUMENT_FOR_NULL(directory_path, NULL); + RCUTILS_CHECK_ALLOCATOR_WITH_MSG( + &allocator, "allocator is invalid", return NULL); + + rcutils_dir_iter_t * iter = (rcutils_dir_iter_t *)allocator.zero_allocate( + 1, sizeof(rcutils_dir_iter_t), allocator.state); + if (NULL == iter) { + return NULL; } - free_dir_list(dir_list, allocator); - return ret; -#else - DIR * dir = NULL; + iter->allocator = allocator; - struct dirent * entry; - do { - dir = opendir(dir_list->path); - if (NULL == dir) { - RCUTILS_SAFE_FWRITE_TO_STDERR_WITH_FORMAT_STRING( - "Can't open directory %s. Error code: %d\n", dir_list->path, errno); - ret = RCUTILS_RET_ERROR; - goto fail; - } + rcutils_dir_iter_state_t * state = (rcutils_dir_iter_state_t *)allocator.zero_allocate( + 1, sizeof(rcutils_dir_iter_state_t), allocator.state); + if (NULL == state) { + RCUTILS_SET_ERROR_MSG( + "Failed to allocate memory.\n"); + goto rcutils_dir_iter_start_fail; + } + iter->state = (void *)state; - // Scan in specified path - // If found directory, add to dir_list - // If found file, calculate file size - while (NULL != (entry = readdir(dir))) { - ret = check_and_calculate_size(entry->d_name, size, max_depth, dir_list, allocator); - if (RCUTILS_RET_OK != ret) { - goto fail; - } +#ifdef _WIN32 + char * search_path = rcutils_join_path(directory_path, "*", allocator); + if (NULL == search_path) { + goto rcutils_dir_iter_start_fail; + } + state->handle = FindFirstFile(search_path, &state->data); + allocator.deallocate(search_path, allocator.state); + if (INVALID_HANDLE_VALUE == state->handle) { + DWORD error = GetLastError(); + if (ERROR_FILE_NOT_FOUND != error || !rcutils_is_directory(directory_path)) { + RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING( + "Can't open directory %s. Error code: %d\n", directory_path, error); + goto rcutils_dir_iter_start_fail; } + } else { + iter->entry_name = state->data.cFileName; + } +#else + state->dir = opendir(directory_path); + if (NULL == state->dir) { + RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING( + "Can't open directory %s. Error code: %d\n", directory_path, errno); + goto rcutils_dir_iter_start_fail; + } + + errno = 0; + struct dirent * entry = readdir(state->dir); + if (NULL != entry) { + iter->entry_name = entry->d_name; + } else if (0 != errno) { + RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING( + "Can't iterate directory %s. Error code: %d\n", directory_path, errno); + goto rcutils_dir_iter_start_fail; + } +#endif - closedir(dir); + return iter; - remove_first_dir_from_list(&dir_list, allocator); - } while (dir_list); +rcutils_dir_iter_start_fail: + rcutils_dir_iter_end(iter); + return NULL; +} - return ret; +bool +rcutils_dir_iter_next(rcutils_dir_iter_t * iter) +{ + RCUTILS_CHECK_ARGUMENT_FOR_NULL(iter, false); + rcutils_dir_iter_state_t * state = (rcutils_dir_iter_state_t *)iter->state; + RCUTILS_CHECK_FOR_NULL_WITH_MSG(state, "iter is invalid", false); -fail: - if (NULL != dir) { - closedir(dir); +#ifdef _WIN32 + if (FindNextFile(state->handle, &state->data)) { + iter->entry_name = state->data.cFileName; + return true; + } + FindClose(state->handle); +#else + struct dirent * entry = readdir(state->dir); + if (NULL != entry) { + iter->entry_name = entry->d_name; + return true; } - free_dir_list(dir_list, allocator); - return ret; #endif + + iter->entry_name = NULL; + return false; } -size_t -rcutils_get_file_size(const char * file_path) +void +rcutils_dir_iter_end(rcutils_dir_iter_t * iter) { - if (!rcutils_is_file(file_path)) { - RCUTILS_SAFE_FWRITE_TO_STDERR_WITH_FORMAT_STRING( - "Path is not a file: %s\n", file_path); - return 0; + if (NULL == iter) { + return; } - struct stat stat_buffer; - int rc = stat(file_path, &stat_buffer); - return rc == 0 ? (size_t)(stat_buffer.st_size) : 0; + rcutils_allocator_t allocator = iter->allocator; + rcutils_dir_iter_state_t * state = (rcutils_dir_iter_state_t *)iter->state; + if (NULL != state) { +#ifdef _WIN32 + FindClose(state->handle); +#else + if (NULL != state->dir) { + closedir(state->dir); + } +#endif + + allocator.deallocate(state, allocator.state); + } + + allocator.deallocate(iter, allocator.state); } #ifdef __cplusplus diff --git a/test/test_filesystem.cpp b/test/test_filesystem.cpp index cfd105a7..8a649f5b 100644 --- a/test/test_filesystem.cpp +++ b/test/test_filesystem.cpp @@ -15,6 +15,7 @@ #include #include +#include "rcutils/error_handling.h" #include "rcutils/filesystem.h" #include "rcutils/get_env.h" @@ -456,6 +457,7 @@ TEST_F(TestFilesystemFixture, calculate_directory_size) { fs.exhaust_file_descriptors(); ret = rcutils_calculate_directory_size(path, &size, g_allocator); EXPECT_EQ(RCUTILS_RET_ERROR, ret); + rcutils_reset_error(); } } @@ -513,6 +515,7 @@ TEST_F(TestFilesystemFixture, calculate_directory_size_with_recursion) { fs.exhaust_file_descriptors(); ret = rcutils_calculate_directory_size_with_recursion(path, 0, &size, g_allocator); EXPECT_EQ(RCUTILS_RET_ERROR, ret); + rcutils_reset_error(); } } @@ -541,3 +544,67 @@ TEST_F(TestFilesystemFixture, calculate_file_size) { g_allocator.deallocate(non_existing_path, g_allocator.state); }); } + +TEST_F(TestFilesystemFixture, directory_iterator) { + char * path = + rcutils_join_path(this->test_path, "dummy_folder", g_allocator); + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( + { + g_allocator.deallocate(path, g_allocator.state); + }); + + rcutils_dir_iter_t * iter = rcutils_dir_iter_start(path, g_allocator); + ASSERT_NE(nullptr, iter); + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( + { + rcutils_dir_iter_end(iter); + }); + + const char * expected[] = { + ".", + "..", + "dummy.dummy", + }; + + do { + size_t i; + for (i = 0; i < sizeof(expected) / sizeof(expected[0]); i++) { + if (nullptr != expected[i] && 0 == strcmp(expected[i], iter->entry_name)) { + expected[i] = nullptr; + break; + } + } + + if (i >= sizeof(expected) / sizeof(expected[0])) { + ADD_FAILURE() << "Unexpected entry " << iter->entry_name << " during iteration"; + } + } while (rcutils_dir_iter_next(iter)); + + for (size_t i = 0; i < sizeof(expected) / sizeof(expected[0]); i++) { + EXPECT_EQ(nullptr, expected[i]) << "Expected entry " << expected[i] << " not found"; + } +} + +TEST_F(TestFilesystemFixture, directory_iterator_non_existing) { + char * path = + rcutils_join_path(this->test_path, "non_existing_folder", g_allocator); + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( + { + g_allocator.deallocate(path, g_allocator.state); + }); + + EXPECT_EQ(nullptr, rcutils_dir_iter_start(path, g_allocator)); + rcutils_reset_error(); +} + +TEST_F(TestFilesystemFixture, directory_iterator_on_file) { + char * path = + rcutils_join_path(this->test_path, "dummy_readable_file.txt", g_allocator); + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( + { + g_allocator.deallocate(path, g_allocator.state); + }); + + EXPECT_EQ(nullptr, rcutils_dir_iter_start(path, g_allocator)); + rcutils_reset_error(); +} From 983776500a49a65ff8d596a272bd477fa29cad2f Mon Sep 17 00:00:00 2001 From: Scott K Logan Date: Wed, 13 May 2020 15:49:12 -0700 Subject: [PATCH 2/7] Add rcutils_list_directory function This change adds a simple function for listing the contents of a directory, and returning the names of those files as a string_array object. Signed-off-by: Scott K Logan --- include/rcutils/filesystem.h | 22 ++++++++++++++ src/filesystem.c | 57 ++++++++++++++++++++++++++++++++++++ test/test_filesystem.cpp | 49 +++++++++++++++++++++++++++++++ 3 files changed, 128 insertions(+) diff --git a/include/rcutils/filesystem.h b/include/rcutils/filesystem.h index 5321801b..74f1b923 100644 --- a/include/rcutils/filesystem.h +++ b/include/rcutils/filesystem.h @@ -26,6 +26,7 @@ extern "C" #include "rcutils/allocator.h" #include "rcutils/macros.h" +#include "rcutils/types.h" #include "rcutils/visibility_control.h" /// Return current working directory. @@ -291,6 +292,27 @@ RCUTILS_PUBLIC void rcutils_dir_iter_end(rcutils_dir_iter_t * iter); +/// List the entries present in a directory. +/** + * This function lists the name of each file or directory present in the given + * directory in an implementation-specific order. + * + * \param[in] directory_path The directory path to list the contents of. + * \param[in] allocator Allocator being used for resources in the string_array. + * \param[out] string_array The object to store the entries into. + * \return `RCUTILS_RET_OK` if successful, or + * \return `RCUTILS_RET_INVALID_ARGUMENT` for invalid arguments, or + * \return `RCUTILS_RET_BAD_ALLOC` if memory allocation fails, or + * \return `RCUTILS_RET_ERROR` if an unknown error occurs + */ +RCUTILS_PUBLIC +RCUTILS_WARN_UNUSED +rcutils_ret_t +rcutils_list_directory( + const char * directory_path, + rcutils_allocator_t allocator, + rcutils_string_array_t * string_array); + #ifdef __cplusplus } #endif diff --git a/src/filesystem.c b/src/filesystem.c index 6f5f79a2..b1e3e7bc 100644 --- a/src/filesystem.c +++ b/src/filesystem.c @@ -45,6 +45,7 @@ extern "C" #include "rcutils/error_handling.h" #include "rcutils/format_string.h" #include "rcutils/get_env.h" +#include "rcutils/logging_macros.h" #include "rcutils/repl_str.h" #include "rcutils/strdup.h" @@ -539,6 +540,62 @@ rcutils_dir_iter_end(rcutils_dir_iter_t * iter) allocator.deallocate(iter, allocator.state); } +rcutils_ret_t +rcutils_list_directory( + const char * directory_path, + rcutils_allocator_t allocator, + rcutils_string_array_t * string_array) +{ + RCUTILS_CHECK_FOR_NULL_WITH_MSG( + directory_path, "directory_path is null", return RCUTILS_RET_INVALID_ARGUMENT); + RCUTILS_CHECK_FOR_NULL_WITH_MSG( + string_array, "string_array is null", return RCUTILS_RET_INVALID_ARGUMENT); + + // Start with 8 entries + rcutils_ret_t ret = rcutils_string_array_init(string_array, 8, &allocator); + if (RCUTILS_RET_OK != ret) { + return ret; + } + + size_t count = 0; + + rcutils_dir_iter_t * iter = rcutils_dir_iter_start(directory_path, allocator); + if (NULL == iter) { + return RCUTILS_RET_ERROR; + } + + do { + if (count >= string_array->size) { + ret = rcutils_string_array_resize(string_array, count * 2); + if (RCUTILS_RET_OK != ret) { + goto fail; + } + } + + string_array->data[count] = rcutils_strdup(iter->entry_name, allocator); + if (NULL == string_array->data[count]) { + goto fail; + } + } while (++count, rcutils_dir_iter_next(iter)); + + // Shrink the array back down + if (count != string_array->size) { + ret = rcutils_string_array_resize(string_array, count); + if (RCUTILS_RET_OK == ret) { + return RCUTILS_RET_OK; + } + } + +fail: + rcutils_dir_iter_end(iter); + + if (RCUTILS_RET_OK != rcutils_string_array_fini(string_array)) { + RCUTILS_LOG_ERROR( + "failed to clean up on error (leaking memory): '%s'", rcutils_get_error_string().str); + } + return ret; +} + #ifdef __cplusplus } #endif diff --git a/test/test_filesystem.cpp b/test/test_filesystem.cpp index 8a649f5b..f4f4610b 100644 --- a/test/test_filesystem.cpp +++ b/test/test_filesystem.cpp @@ -23,6 +23,10 @@ #include "./mocking_utils/filesystem.hpp" +#ifdef _WIN32 + #define strdup _strdup +#endif + static rcutils_allocator_t g_allocator = rcutils_get_default_allocator(); class TestFilesystemFixture : public ::testing::Test @@ -608,3 +612,48 @@ TEST_F(TestFilesystemFixture, directory_iterator_on_file) { EXPECT_EQ(nullptr, rcutils_dir_iter_start(path, g_allocator)); rcutils_reset_error(); } + +TEST_F(TestFilesystemFixture, list_directory) { + char * path = + rcutils_join_path(this->test_path, "dummy_folder", g_allocator); + ASSERT_NE(nullptr, path); + + rcutils_string_array_t actual_contents; + rcutils_ret_t ret = rcutils_list_directory(nullptr, g_allocator, &actual_contents); + EXPECT_EQ(RCUTILS_RET_INVALID_ARGUMENT, ret); + rcutils_reset_error(); + + ret = rcutils_list_directory(path, g_allocator, nullptr); + EXPECT_EQ(RCUTILS_RET_INVALID_ARGUMENT, ret); + rcutils_reset_error(); + + ret = rcutils_list_directory(path, g_allocator, &actual_contents); + ASSERT_EQ(RCUTILS_RET_OK, ret); + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( + { + if (RCUTILS_RET_OK != rcutils_string_array_fini(&actual_contents)) { + FAIL(); + } + }); + + ret = rcutils_string_array_sort(&actual_contents); + ASSERT_EQ(RCUTILS_RET_OK, ret); + + rcutils_string_array_t expected_contents; + ret = rcutils_string_array_init(&expected_contents, 3, &g_allocator); + ASSERT_EQ(RCUTILS_RET_OK, ret); + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( + { + if (RCUTILS_RET_OK != rcutils_string_array_fini(&expected_contents)) { + FAIL(); + } + }); + expected_contents.data[0] = strdup("."); + expected_contents.data[1] = strdup(".."); + expected_contents.data[2] = strdup("dummy.dummy"); + + int res; + ret = rcutils_string_array_cmp(&expected_contents, &actual_contents, &res); + ASSERT_EQ(RCUTILS_RET_OK, ret); + EXPECT_EQ(0, res); +} From e63db31cebbe84ac84608a22651c80105c7b5584 Mon Sep 17 00:00:00 2001 From: Scott K Logan Date: Mon, 11 Jan 2021 18:59:46 -0800 Subject: [PATCH 3/7] Move rcutils_get_file_size to the end to clean up diff Signed-off-by: Scott K Logan --- src/filesystem.c | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/src/filesystem.c b/src/filesystem.c index b1e3e7bc..f9106bc1 100644 --- a/src/filesystem.c +++ b/src/filesystem.c @@ -411,20 +411,6 @@ rcutils_calculate_directory_size_with_recursion( return ret; } -size_t -rcutils_get_file_size(const char * file_path) -{ - if (!rcutils_is_file(file_path)) { - RCUTILS_SAFE_FWRITE_TO_STDERR_WITH_FORMAT_STRING( - "Path is not a file: %s\n", file_path); - return 0; - } - - struct stat stat_buffer; - int rc = stat(file_path, &stat_buffer); - return rc == 0 ? (size_t)(stat_buffer.st_size) : 0; -} - rcutils_dir_iter_t * rcutils_dir_iter_start(const char * directory_path, const rcutils_allocator_t allocator) { @@ -596,6 +582,20 @@ rcutils_list_directory( return ret; } +size_t +rcutils_get_file_size(const char * file_path) +{ + if (!rcutils_is_file(file_path)) { + RCUTILS_SAFE_FWRITE_TO_STDERR_WITH_FORMAT_STRING( + "Path is not a file: %s\n", file_path); + return 0; + } + + struct stat stat_buffer; + int rc = stat(file_path, &stat_buffer); + return rc == 0 ? (size_t)(stat_buffer.st_size) : 0; +} + #ifdef __cplusplus } #endif From 1242828148b629393bed201671c3026928dbae8e Mon Sep 17 00:00:00 2001 From: Scott K Logan Date: Mon, 11 Jan 2021 19:56:25 -0800 Subject: [PATCH 4/7] Doh, this is C++ so we can clean this up quite a bit Signed-off-by: Scott K Logan --- test/test_filesystem.cpp | 29 +++++++++++------------------ 1 file changed, 11 insertions(+), 18 deletions(-) diff --git a/test/test_filesystem.cpp b/test/test_filesystem.cpp index f4f4610b..2a445a49 100644 --- a/test/test_filesystem.cpp +++ b/test/test_filesystem.cpp @@ -13,6 +13,7 @@ // limitations under the License. #include +#include #include #include "rcutils/error_handling.h" @@ -557,6 +558,12 @@ TEST_F(TestFilesystemFixture, directory_iterator) { g_allocator.deallocate(path, g_allocator.state); }); + std::set expected { + ".", + "..", + "dummy.dummy", + }; + rcutils_dir_iter_t * iter = rcutils_dir_iter_start(path, g_allocator); ASSERT_NE(nullptr, iter); OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( @@ -564,28 +571,14 @@ TEST_F(TestFilesystemFixture, directory_iterator) { rcutils_dir_iter_end(iter); }); - const char * expected[] = { - ".", - "..", - "dummy.dummy", - }; - do { - size_t i; - for (i = 0; i < sizeof(expected) / sizeof(expected[0]); i++) { - if (nullptr != expected[i] && 0 == strcmp(expected[i], iter->entry_name)) { - expected[i] = nullptr; - break; - } - } - - if (i >= sizeof(expected) / sizeof(expected[0])) { - ADD_FAILURE() << "Unexpected entry " << iter->entry_name << " during iteration"; + if (1u != expected.erase(iter->entry_name)) { + ADD_FAILURE() << "Unexpected entry '" << iter->entry_name << "' was enumerated"; } } while (rcutils_dir_iter_next(iter)); - for (size_t i = 0; i < sizeof(expected) / sizeof(expected[0]); i++) { - EXPECT_EQ(nullptr, expected[i]) << "Expected entry " << expected[i] << " not found"; + for (std::string missing : expected) { + ADD_FAILURE() << "Expected entry '" << missing << "' was not enumerated"; } } From 58d66e831945b177ae7b7b59c257fc8d6f70631e Mon Sep 17 00:00:00 2001 From: Scott K Logan Date: Mon, 11 Jan 2021 20:15:07 -0800 Subject: [PATCH 5/7] Drop some unnecessary stuff Signed-off-by: Scott K Logan --- src/filesystem.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/filesystem.c b/src/filesystem.c index f9106bc1..303a174e 100644 --- a/src/filesystem.c +++ b/src/filesystem.c @@ -377,8 +377,7 @@ rcutils_calculate_directory_size_with_recursion( dir_list->path = rcutils_strdup(directory_path, allocator); if (NULL == dir_list->path) { RCUTILS_SAFE_FWRITE_TO_STDERR("Failed to duplicate directory path !\n"); - ret = RCUTILS_RET_BAD_ALLOC; - goto fail; + return RCUTILS_RET_BAD_ALLOC; } *size = 0; @@ -398,7 +397,6 @@ rcutils_calculate_directory_size_with_recursion( } while (rcutils_dir_iter_next(iter)); rcutils_dir_iter_end(iter); - iter = NULL; remove_first_dir_from_list(&dir_list, allocator); } while (dir_list); From e5240903f65eb60b4d3673a2e314a537f70456da Mon Sep 17 00:00:00 2001 From: Scott K Logan Date: Tue, 12 Jan 2021 09:18:06 -0800 Subject: [PATCH 6/7] Revert "Add rcutils_list_directory function" This reverts commit 983776500a49a65ff8d596a272bd477fa29cad2f. Signed-off-by: Scott K Logan --- include/rcutils/filesystem.h | 22 -------------- src/filesystem.c | 57 ------------------------------------ test/test_filesystem.cpp | 49 ------------------------------- 3 files changed, 128 deletions(-) diff --git a/include/rcutils/filesystem.h b/include/rcutils/filesystem.h index 74f1b923..5321801b 100644 --- a/include/rcutils/filesystem.h +++ b/include/rcutils/filesystem.h @@ -26,7 +26,6 @@ extern "C" #include "rcutils/allocator.h" #include "rcutils/macros.h" -#include "rcutils/types.h" #include "rcutils/visibility_control.h" /// Return current working directory. @@ -292,27 +291,6 @@ RCUTILS_PUBLIC void rcutils_dir_iter_end(rcutils_dir_iter_t * iter); -/// List the entries present in a directory. -/** - * This function lists the name of each file or directory present in the given - * directory in an implementation-specific order. - * - * \param[in] directory_path The directory path to list the contents of. - * \param[in] allocator Allocator being used for resources in the string_array. - * \param[out] string_array The object to store the entries into. - * \return `RCUTILS_RET_OK` if successful, or - * \return `RCUTILS_RET_INVALID_ARGUMENT` for invalid arguments, or - * \return `RCUTILS_RET_BAD_ALLOC` if memory allocation fails, or - * \return `RCUTILS_RET_ERROR` if an unknown error occurs - */ -RCUTILS_PUBLIC -RCUTILS_WARN_UNUSED -rcutils_ret_t -rcutils_list_directory( - const char * directory_path, - rcutils_allocator_t allocator, - rcutils_string_array_t * string_array); - #ifdef __cplusplus } #endif diff --git a/src/filesystem.c b/src/filesystem.c index 303a174e..0ab70773 100644 --- a/src/filesystem.c +++ b/src/filesystem.c @@ -45,7 +45,6 @@ extern "C" #include "rcutils/error_handling.h" #include "rcutils/format_string.h" #include "rcutils/get_env.h" -#include "rcutils/logging_macros.h" #include "rcutils/repl_str.h" #include "rcutils/strdup.h" @@ -524,62 +523,6 @@ rcutils_dir_iter_end(rcutils_dir_iter_t * iter) allocator.deallocate(iter, allocator.state); } -rcutils_ret_t -rcutils_list_directory( - const char * directory_path, - rcutils_allocator_t allocator, - rcutils_string_array_t * string_array) -{ - RCUTILS_CHECK_FOR_NULL_WITH_MSG( - directory_path, "directory_path is null", return RCUTILS_RET_INVALID_ARGUMENT); - RCUTILS_CHECK_FOR_NULL_WITH_MSG( - string_array, "string_array is null", return RCUTILS_RET_INVALID_ARGUMENT); - - // Start with 8 entries - rcutils_ret_t ret = rcutils_string_array_init(string_array, 8, &allocator); - if (RCUTILS_RET_OK != ret) { - return ret; - } - - size_t count = 0; - - rcutils_dir_iter_t * iter = rcutils_dir_iter_start(directory_path, allocator); - if (NULL == iter) { - return RCUTILS_RET_ERROR; - } - - do { - if (count >= string_array->size) { - ret = rcutils_string_array_resize(string_array, count * 2); - if (RCUTILS_RET_OK != ret) { - goto fail; - } - } - - string_array->data[count] = rcutils_strdup(iter->entry_name, allocator); - if (NULL == string_array->data[count]) { - goto fail; - } - } while (++count, rcutils_dir_iter_next(iter)); - - // Shrink the array back down - if (count != string_array->size) { - ret = rcutils_string_array_resize(string_array, count); - if (RCUTILS_RET_OK == ret) { - return RCUTILS_RET_OK; - } - } - -fail: - rcutils_dir_iter_end(iter); - - if (RCUTILS_RET_OK != rcutils_string_array_fini(string_array)) { - RCUTILS_LOG_ERROR( - "failed to clean up on error (leaking memory): '%s'", rcutils_get_error_string().str); - } - return ret; -} - size_t rcutils_get_file_size(const char * file_path) { diff --git a/test/test_filesystem.cpp b/test/test_filesystem.cpp index 2a445a49..e263128a 100644 --- a/test/test_filesystem.cpp +++ b/test/test_filesystem.cpp @@ -24,10 +24,6 @@ #include "./mocking_utils/filesystem.hpp" -#ifdef _WIN32 - #define strdup _strdup -#endif - static rcutils_allocator_t g_allocator = rcutils_get_default_allocator(); class TestFilesystemFixture : public ::testing::Test @@ -605,48 +601,3 @@ TEST_F(TestFilesystemFixture, directory_iterator_on_file) { EXPECT_EQ(nullptr, rcutils_dir_iter_start(path, g_allocator)); rcutils_reset_error(); } - -TEST_F(TestFilesystemFixture, list_directory) { - char * path = - rcutils_join_path(this->test_path, "dummy_folder", g_allocator); - ASSERT_NE(nullptr, path); - - rcutils_string_array_t actual_contents; - rcutils_ret_t ret = rcutils_list_directory(nullptr, g_allocator, &actual_contents); - EXPECT_EQ(RCUTILS_RET_INVALID_ARGUMENT, ret); - rcutils_reset_error(); - - ret = rcutils_list_directory(path, g_allocator, nullptr); - EXPECT_EQ(RCUTILS_RET_INVALID_ARGUMENT, ret); - rcutils_reset_error(); - - ret = rcutils_list_directory(path, g_allocator, &actual_contents); - ASSERT_EQ(RCUTILS_RET_OK, ret); - OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( - { - if (RCUTILS_RET_OK != rcutils_string_array_fini(&actual_contents)) { - FAIL(); - } - }); - - ret = rcutils_string_array_sort(&actual_contents); - ASSERT_EQ(RCUTILS_RET_OK, ret); - - rcutils_string_array_t expected_contents; - ret = rcutils_string_array_init(&expected_contents, 3, &g_allocator); - ASSERT_EQ(RCUTILS_RET_OK, ret); - OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( - { - if (RCUTILS_RET_OK != rcutils_string_array_fini(&expected_contents)) { - FAIL(); - } - }); - expected_contents.data[0] = strdup("."); - expected_contents.data[1] = strdup(".."); - expected_contents.data[2] = strdup("dummy.dummy"); - - int res; - ret = rcutils_string_array_cmp(&expected_contents, &actual_contents, &res); - ASSERT_EQ(RCUTILS_RET_OK, ret); - EXPECT_EQ(0, res); -} From cbda7e636ed2c837ae8bc23c43282cc29e99162e Mon Sep 17 00:00:00 2001 From: Scott K Logan Date: Thu, 14 Jan 2021 19:16:27 -0800 Subject: [PATCH 7/7] Update include/rcutils/filesystem.h Signed-off-by: Scott K Logan Co-authored-by: tomoya --- include/rcutils/filesystem.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/rcutils/filesystem.h b/include/rcutils/filesystem.h index 5321801b..8b802715 100644 --- a/include/rcutils/filesystem.h +++ b/include/rcutils/filesystem.h @@ -256,7 +256,7 @@ typedef struct rcutils_dir_iter_t /// Begin iterating over the contents of the specified directory. /* - * This function is used to list the file and directories that are contained in + * This function is used to list the files and directories that are contained in * a specified directory. The structure returned by it must be deallocated using * ::rcutils_dir_iter_end when the iteration is completed. The name of the * enumerated entry is stored in the `entry_name` member of the returned object,