From e17553089eea728f4e7f0f75250694283f1c6bb7 Mon Sep 17 00:00:00 2001 From: Joseph Hickey Date: Mon, 28 Dec 2020 13:54:23 -0500 Subject: [PATCH] Fix #580, improve FS_BASED mounts on VxWorks The mount/unmount implementation was not really checking for and handling this mapping type. To be consistent with POSIX it should also create a directory if it does not already exist. --- src/os/inc/osapi-filesys.h | 10 +++ src/os/vxworks/src/os-impl-filesys.c | 83 +++++++++++++++---- .../vxworks/src/coveragetest-filesys.c | 34 ++++++++ 3 files changed, 110 insertions(+), 17 deletions(-) diff --git a/src/os/inc/osapi-filesys.h b/src/os/inc/osapi-filesys.h index 8b3d6797c..94c7ea035 100644 --- a/src/os/inc/osapi-filesys.h +++ b/src/os/inc/osapi-filesys.h @@ -56,6 +56,16 @@ typedef struct * This mimics the behavior of a "FS_BASED" entry in the VolumeTable but is registered * at runtime. It is intended to be called by the PSP/BSP prior to starting the application. * + * @note OSAL virtual mount points are required to be a single, non-empty top-level directory + * name. Virtual path names always follow the form ///. + * Only the relative path may be omitted/empty (i.e. //) but the + * virtual mount point must be present and not an empty string. In particular this means + * it is not possible to directly refer to files in the "root" of the native file system + * from OSAL. However it is possible to create a virtual map to the root, such as by calling: + * + * OS_FileSysAddFixedMap(&fs_id, "/", "/root"); + * + * * @param[out] filesys_id A non-zero OSAL ID reflecting the file system * @param[in] phys_path The native system directory (an existing mount point) * @param[in] virt_path The virtual mount point of this filesystem diff --git a/src/os/vxworks/src/os-impl-filesys.c b/src/os/vxworks/src/os-impl-filesys.c index 5a7080c6e..5352e8563 100644 --- a/src/os/vxworks/src/os-impl-filesys.c +++ b/src/os/vxworks/src/os-impl-filesys.c @@ -32,6 +32,7 @@ #include "os-vxworks.h" #include "os-impl-filesys.h" +#include "os-impl-dirs.h" #include "os-shared-filesys.h" #include "os-shared-idmap.h" @@ -295,22 +296,62 @@ int32 OS_FileSysMountVolume_Impl(const OS_object_token_t *token) OS_filesys_internal_record_t *local; int32 status; int fd; + struct stat stat_buf; local = OS_OBJECT_TABLE_GET(OS_filesys_table, *token); /* - * Calling open() on the physical device path - * mounts the device. + * For FS-based mounts, these are just a map to a some other + * directory in the filesystem. + * + * If it does exist then make sure it is actually a directory. + * If it does not exist then attempt to create it. */ - fd = open(local->system_mountpt, O_RDONLY, 0644); - if (fd < 0) + if (local->fstype == OS_FILESYS_TYPE_FS_BASED) { - status = OS_ERROR; + if (stat(local->system_mountpt, &stat_buf) == 0) + { + if (S_ISDIR(stat_buf.st_mode)) + { + /* mount point exists */ + status = OS_SUCCESS; + } + else + { + OS_DEBUG("%s is not a directory\n", local->system_mountpt); + status = OS_FS_ERR_PATH_INVALID; + } + } + else + { + if (mkdir(local->system_mountpt, 0775) == 0) + { + /* directory created OK */ + status = OS_SUCCESS; + } + else + { + OS_DEBUG("mkdir(%s): errno=%d\n", local->system_mountpt, errnoGet()); + status = OS_FS_ERR_DRIVE_NOT_CREATED; + } + } } else { - status = OS_SUCCESS; - close(fd); + /* + * For all other (non-FS_BASED) filesystem types, + * Calling open() on the physical device path mounts the device. + */ + fd = open(local->system_mountpt, O_RDONLY, 0644); + if (fd < 0) + { + status = OS_ERROR; + } + else + { + status = OS_SUCCESS; + close(fd); + } } return status; @@ -333,26 +374,34 @@ int32 OS_FileSysUnmountVolume_Impl(const OS_object_token_t *token) local = OS_OBJECT_TABLE_GET(OS_filesys_table, *token); - /* - ** vxWorks uses an ioctl to unmount - */ - fd = open(local->system_mountpt, O_RDONLY, 0644); - if (fd < 0) + if (local->fstype == OS_FILESYS_TYPE_FS_BASED) { - status = OS_ERROR; + /* unmount is a no-op on FS-based mounts - it is just a directory map */ + status = OS_SUCCESS; } else { - if (ioctl(fd, FIOUNMOUNT, 0) < 0) + /* + ** vxWorks uses an ioctl to unmount + */ + fd = open(local->system_mountpt, O_RDONLY, 0644); + if (fd < 0) { status = OS_ERROR; } else { - status = OS_SUCCESS; - } + if (ioctl(fd, FIOUNMOUNT, 0) < 0) + { + status = OS_ERROR; + } + else + { + status = OS_SUCCESS; + } - close(fd); + close(fd); + } } return status; diff --git a/src/unit-test-coverage/vxworks/src/coveragetest-filesys.c b/src/unit-test-coverage/vxworks/src/coveragetest-filesys.c index 52c2c69cf..de2998415 100644 --- a/src/unit-test-coverage/vxworks/src/coveragetest-filesys.c +++ b/src/unit-test-coverage/vxworks/src/coveragetest-filesys.c @@ -130,12 +130,38 @@ void Test_OS_FileSysMountVolume_Impl(void) * int32 OS_FileSysMountVolume_Impl (uint32 filesys_id) */ OS_object_token_t token = UT_TOKEN_0; + struct OCS_stat statbuf; + + memset(&OS_filesys_table[0], 0, sizeof(OS_filesys_table[0])); + OS_filesys_table[0].fstype = OS_FILESYS_TYPE_NORMAL_DISK; + strcpy(OS_filesys_table[0].system_mountpt, "/ut"); OSAPI_TEST_FUNCTION_RC(OS_FileSysMountVolume_Impl(&token), OS_SUCCESS); UT_SetDefaultReturnValue(UT_KEY(OCS_open), -1); OSAPI_TEST_FUNCTION_RC(OS_FileSysMountVolume_Impl(&token), OS_ERROR); UT_ClearForceFail(UT_KEY(OCS_open)); + + /* Additional cases for the FS_BASED handling */ + OS_filesys_table[0].fstype = OS_FILESYS_TYPE_FS_BASED; + + /* Mount dir does not exist but can be created */ + UT_SetDefaultReturnValue(UT_KEY(OCS_stat), -1); + OSAPI_TEST_FUNCTION_RC(OS_FileSysMountVolume_Impl(&token), OS_SUCCESS); + + /* Mount dir does not exist and cannot be created */ + UT_SetDeferredRetcode(UT_KEY(OCS_mkdir), 1, -1); + OSAPI_TEST_FUNCTION_RC(OS_FileSysMountVolume_Impl(&token), OS_FS_ERR_DRIVE_NOT_CREATED); + + /* Mount dir does exist but not a directory */ + UT_ClearForceFail(UT_KEY(OCS_stat)); + OSAPI_TEST_FUNCTION_RC(OS_FileSysMountVolume_Impl(&token), OS_FS_ERR_PATH_INVALID); + + /* Mount dir does exist and is a directory */ + memset(&statbuf, 0, sizeof(statbuf)); + statbuf.st_mode = OCS_S_IFDIR; + UT_SetDataBuffer(UT_KEY(OCS_stat), &statbuf, sizeof(statbuf), false); + OSAPI_TEST_FUNCTION_RC(OS_FileSysMountVolume_Impl(&token), OS_SUCCESS); } void Test_OS_FileSysUnmountVolume_Impl(void) @@ -145,6 +171,10 @@ void Test_OS_FileSysUnmountVolume_Impl(void) */ OS_object_token_t token = UT_TOKEN_0; + memset(&OS_filesys_table[0], 0, sizeof(OS_filesys_table[0])); + OS_filesys_table[0].fstype = OS_FILESYS_TYPE_NORMAL_DISK; + strcpy(OS_filesys_table[0].system_mountpt, "/ut"); + OSAPI_TEST_FUNCTION_RC(OS_FileSysUnmountVolume_Impl(&token), OS_SUCCESS); UT_SetDefaultReturnValue(UT_KEY(OCS_open), -1); @@ -154,6 +184,10 @@ void Test_OS_FileSysUnmountVolume_Impl(void) UT_SetDefaultReturnValue(UT_KEY(OCS_ioctl), -1); OSAPI_TEST_FUNCTION_RC(OS_FileSysUnmountVolume_Impl(&token), OS_ERROR); UT_ClearForceFail(UT_KEY(OCS_ioctl)); + + /* Additional cases for the FS_BASED handling (no op on unmount) */ + OS_filesys_table[0].fstype = OS_FILESYS_TYPE_FS_BASED; + OSAPI_TEST_FUNCTION_RC(OS_FileSysUnmountVolume_Impl(&token), OS_SUCCESS); } void Test_OS_FileSysStatVolume_Impl(void)