From 8f5d6bc3bb4fce7e3d19c18c906ceafe1c348ae7 Mon Sep 17 00:00:00 2001 From: Chris Lalancette Date: Tue, 4 Jun 2024 15:04:35 -0400 Subject: [PATCH] Fix filesystem iteration on Windows (#469) * Get rid of unnecessary casts in rcutils_dir_iter_t. Instead of using a void pointer, use the forward-declared pointer type. While this won't make a difference for the generated code, it will let us remove unnecessary casts. * Remove unnecessary FindClose on error. In particular, if we failed to find the next file on Windows, we shouldn't necessarily close the handle; that's the job of rcutils_dir_iter_end. * Remove completely unnecessary #ifdef __cplusplus header. Signed-off-by: Chris Lalancette --- include/rcutils/filesystem.h | 4 ++- src/filesystem.c | 51 ++++++++++++++---------------------- 2 files changed, 23 insertions(+), 32 deletions(-) diff --git a/include/rcutils/filesystem.h b/include/rcutils/filesystem.h index 20dd2c7c..91ef51de 100644 --- a/include/rcutils/filesystem.h +++ b/include/rcutils/filesystem.h @@ -245,6 +245,8 @@ RCUTILS_PUBLIC size_t rcutils_get_file_size(const char * file_path); +struct rcutils_dir_iter_state_s; + /// An iterator used for enumerating directory contents typedef struct rcutils_dir_iter_s { @@ -253,7 +255,7 @@ typedef struct rcutils_dir_iter_s /// The allocator used internally by iteration functions rcutils_allocator_t allocator; /// The platform-specific iteration state - void * state; + struct rcutils_dir_iter_state_s * state; } rcutils_dir_iter_t; /// Begin iterating over the contents of the specified directory. diff --git a/src/filesystem.c b/src/filesystem.c index d764f99c..25c71d79 100644 --- a/src/filesystem.c +++ b/src/filesystem.c @@ -12,10 +12,6 @@ // See the License for the specific language governing permissions and // limitations under the License. -#ifdef __cplusplus -extern "C" -{ -#endif #include "rcutils/filesystem.h" #include @@ -54,7 +50,7 @@ extern "C" # define RCUTILS_PATH_DELIMITER "/" #endif // _WIN32 -typedef struct rcutils_dir_iter_state_t +typedef struct rcutils_dir_iter_state_s { #ifdef _WIN32 HANDLE handle; @@ -417,30 +413,29 @@ rcutils_dir_iter_start(const char * directory_path, const rcutils_allocator_t al RCUTILS_CHECK_ALLOCATOR_WITH_MSG( &allocator, "allocator is invalid", return NULL); - rcutils_dir_iter_t * iter = (rcutils_dir_iter_t *)allocator.zero_allocate( + rcutils_dir_iter_t * iter = allocator.zero_allocate( 1, sizeof(rcutils_dir_iter_t), allocator.state); if (NULL == iter) { return NULL; } iter->allocator = allocator; - rcutils_dir_iter_state_t * state = (rcutils_dir_iter_state_t *)allocator.zero_allocate( + iter->state = allocator.zero_allocate( 1, sizeof(rcutils_dir_iter_state_t), allocator.state); - if (NULL == state) { + if (NULL == iter->state) { RCUTILS_SET_ERROR_MSG( "Failed to allocate memory.\n"); goto rcutils_dir_iter_start_fail; } - iter->state = (void *)state; #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); + iter->state->handle = FindFirstFile(search_path, &iter->state->data); allocator.deallocate(search_path, allocator.state); - if (INVALID_HANDLE_VALUE == state->handle) { + if (INVALID_HANDLE_VALUE == iter->state->handle) { DWORD error = GetLastError(); if (ERROR_FILE_NOT_FOUND != error || !rcutils_is_directory(directory_path)) { RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING( @@ -448,18 +443,18 @@ rcutils_dir_iter_start(const char * directory_path, const rcutils_allocator_t al goto rcutils_dir_iter_start_fail; } } else { - iter->entry_name = state->data.cFileName; + iter->entry_name = iter->state->data.cFileName; } #else - state->dir = opendir(directory_path); - if (NULL == state->dir) { + iter->state->dir = opendir(directory_path); + if (NULL == iter->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); + struct dirent * entry = readdir(iter->state->dir); if (NULL != entry) { iter->entry_name = entry->d_name; } else if (0 != errno) { @@ -480,17 +475,15 @@ 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); + RCUTILS_CHECK_FOR_NULL_WITH_MSG(iter->state, "iter is invalid", false); #ifdef _WIN32 - if (FindNextFile(state->handle, &state->data)) { - iter->entry_name = state->data.cFileName; + if (FindNextFile(iter->state->handle, &iter->state->data)) { + iter->entry_name = iter->state->data.cFileName; return true; } - FindClose(state->handle); #else - struct dirent * entry = readdir(state->dir); + struct dirent * entry = readdir(iter->state->dir); if (NULL != entry) { iter->entry_name = entry->d_name; return true; @@ -511,17 +504,17 @@ rcutils_dir_iter_end(rcutils_dir_iter_t * iter) rcutils_allocator_t allocator = iter->allocator; RCUTILS_CHECK_ALLOCATOR_WITH_MSG( &allocator, "allocator is invalid", return ); - rcutils_dir_iter_state_t * state = (rcutils_dir_iter_state_t *)iter->state; - if (NULL != state) { + + if (NULL != iter->state) { #ifdef _WIN32 - FindClose(state->handle); + FindClose(iter->state->handle); #else - if (NULL != state->dir) { - closedir(state->dir); + if (NULL != iter->state->dir) { + closedir(iter->state->dir); } #endif - allocator.deallocate(state, allocator.state); + allocator.deallocate(iter->state, allocator.state); } allocator.deallocate(iter, allocator.state); @@ -540,7 +533,3 @@ rcutils_get_file_size(const char * file_path) int rc = stat(file_path, &stat_buffer); return rc == 0 ? (size_t)(stat_buffer.st_size) : 0; } - -#ifdef __cplusplus -} -#endif