From 1c36569232264983974320b9c6335477929707e5 Mon Sep 17 00:00:00 2001 From: Joseph Hickey Date: Thu, 14 May 2020 14:41:22 -0400 Subject: [PATCH 1/4] Fix #367, Deprecate OS_VolumeTable objects Remove the OS_VolumeTable definition from all BSPs, but provide a default (empty) one to support linking when OMIT_DEPRECATED is not set. --- CMakeLists.txt | 26 ++++---- src/bsp/mcp750-vxworks/CMakeLists.txt | 1 - src/bsp/mcp750-vxworks/build_options.cmake | 3 - src/bsp/mcp750-vxworks/src/bsp_voltab.c | 60 ----------------- src/bsp/pc-linux/CMakeLists.txt | 1 - src/bsp/pc-linux/build_options.cmake | 5 -- src/bsp/pc-linux/src/bsp_start.c | 32 --------- src/bsp/pc-linux/src/bsp_voltab.c | 49 -------------- src/bsp/pc-rtems/CMakeLists.txt | 1 - src/bsp/pc-rtems/build_options.cmake | 5 -- src/bsp/pc-rtems/src/bsp_start.c | 66 +++++++------------ src/bsp/pc-rtems/src/bsp_voltab.c | 52 --------------- src/bsp/pc-rtems/src/pcrtems_bsp_internal.h | 5 ++ src/bsp/shared/inc/bsp-impl.h | 5 +- src/bsp/shared/src/bsp_default_voltab.c | 62 +++++++++++++++++ src/os/inc/osapi-os-filesys.h | 20 ++++-- src/os/shared/inc/os-shared-filesys.h | 4 +- src/os/shared/src/osapi-filesys.c | 49 +++++++------- .../shared/src/coveragetest-filesys.c | 46 ------------- .../osfile-test/ut_osfile_dirio_test.h | 2 +- .../osfilesys-test/ut_osfilesys_diskio_test.c | 8 +-- .../osfilesys-test/ut_osfilesys_diskio_test.h | 2 +- src/ut-stubs/utstub-helpers.c | 2 +- 23 files changed, 160 insertions(+), 346 deletions(-) delete mode 100644 src/bsp/mcp750-vxworks/src/bsp_voltab.c delete mode 100644 src/bsp/pc-linux/src/bsp_voltab.c delete mode 100644 src/bsp/pc-rtems/src/bsp_voltab.c create mode 100644 src/bsp/shared/src/bsp_default_voltab.c diff --git a/CMakeLists.txt b/CMakeLists.txt index e378f56ce..2a34d257b 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -30,11 +30,6 @@ # # This also exports the following variables: # -# OSAL_BSP_STAGING_INSTALL_DIR : the location of the staging directory -# for the selected OSAL BSP. This can be used in -# post-build rules or "install()" commands to stage -# binary or data files for execution. -# # UT_COVERAGE_COMPILE_FLAGS : Compiler flags that must be used to # instrument code for coverage testing # UT_COVERAGE_LINK_FLAGS : Linker flags that must be used to @@ -161,13 +156,22 @@ if (OSAL_BSP_INCLUDE_DIRECTORIES) include_directories(${OSAL_BSP_INCLUDE_DIRECTORIES}) endif (OSAL_BSP_INCLUDE_DIRECTORIES) +set(BSP_SRCLIST + src/bsp/shared/src/osapi-bsp.c + src/bsp/shared/src/bsp_default_app_run.c + src/bsp/shared/src/bsp_default_app_startup.c + src/bsp/shared/src/bsp_default_symtab.c +) + +if (NOT OMIT_DEPRECATED) + list(APPEND BSP_SRCLIST + src/bsp/shared/src/bsp_default_voltab.c + ) +endif (NOT OMIT_DEPRECATED) # Define the external "osal_bsp" static library target add_library(osal_bsp STATIC - src/bsp/shared/src/osapi-bsp.c - src/bsp/shared/src/bsp_default_app_run.c - src/bsp/shared/src/bsp_default_app_startup.c - src/bsp/shared/src/bsp_default_symtab.c + ${BSP_SRCLIST} $ ) @@ -337,10 +341,6 @@ endif (ENABLE_UNIT_TESTS) # This is conditional to avoid warnings in a standalone build. get_directory_property(HAS_PARENT PARENT_DIRECTORY) if (HAS_PARENT) - # export the "OSAL_BSP_STAGING_INSTALL_DIR" to the parent build, so this can be - # used in "install" commands as necessary for the files needed at runtime - set(OSAL_BSP_STAGING_INSTALL_DIR "${OSAL_BSP_STAGING_INSTALL_DIR}" PARENT_SCOPE) - # Export the UT coverage compiler/linker flags to the parent build. # These flags are based on the target system type and should be used # when building code intended for coverage analysis. diff --git a/src/bsp/mcp750-vxworks/CMakeLists.txt b/src/bsp/mcp750-vxworks/CMakeLists.txt index 37e906696..a73a2d25d 100644 --- a/src/bsp/mcp750-vxworks/CMakeLists.txt +++ b/src/bsp/mcp750-vxworks/CMakeLists.txt @@ -6,7 +6,6 @@ add_library(osal_mcp750-vxworks_impl OBJECT src/bsp_start.c - src/bsp_voltab.c src/bsp_console.c ) diff --git a/src/bsp/mcp750-vxworks/build_options.cmake b/src/bsp/mcp750-vxworks/build_options.cmake index d9b097934..3e3c71751 100644 --- a/src/bsp/mcp750-vxworks/build_options.cmake +++ b/src/bsp/mcp750-vxworks/build_options.cmake @@ -4,8 +4,5 @@ # ########################################################################## -# This indicates where to stage target binaries created during the build -set(OSAL_BSP_STAGING_INSTALL_DIR "CF:0") - # The "-u" switch is required to ensure that "ldppc" pulls in the OS_BSPMain entry point target_link_libraries(osal_bsp -uOS_BSPMain) diff --git a/src/bsp/mcp750-vxworks/src/bsp_voltab.c b/src/bsp/mcp750-vxworks/src/bsp_voltab.c deleted file mode 100644 index bb25283b9..000000000 --- a/src/bsp/mcp750-vxworks/src/bsp_voltab.c +++ /dev/null @@ -1,60 +0,0 @@ -/* -** File : bsp_voltab.c -** Author : Nicholas Yanchik / GSFC Code 582 -** -** This is governed by the NASA Open Source Agreement and may be used, -** distributed and modified only pursuant to the terms of that agreement. -** -** Copyright (c) 2004-2006, United States government as represented by the -** administrator of the National Aeronautics Space Administration. -** All rights reserved. -** -** -** BSP Volume table for file systems -*/ - -/**************************************************************************************** - INCLUDE FILES -****************************************************************************************/ -#include "common_types.h" -#include "osapi.h" - - -/* -** volume table. -*/ -OS_VolumeInfo_t OS_VolumeTable [NUM_TABLE_ENTRIES] = -{ -/* Dev Name Phys Dev Vol Type Volatile? Free? IsMounted? Volname MountPt BlockSz */ - -/* RAM Disk */ -{"/ramdev0", " ", RAM_DISK, true, true, false, " ", " ", 0 }, - -/* non-volatile Disk -- Auto-Mapped to an existing CF disk */ -{"/eedev0", "CF:0", FS_BASED, false, false, true, "CF", "/cf", 512 }, - -/* -** Spare RAM disks to be used for SSR and other RAM disks -*/ -{"/ramdev1", " ", RAM_DISK, true, true, false, " ", " ", 0 }, -{"/ramdev2", " ", RAM_DISK, true, true, false, " ", " ", 0 }, -{"/ramdev3", " ", RAM_DISK, true, true, false, " ", " ", 0 }, -{"/ramdev4", " ", RAM_DISK, true, true, false, " ", " ", 0 }, -{"/ramdev5", " ", RAM_DISK, true, true, false, " ", " ", 0 }, - -/* -** Hard disk mappings -*/ -{"/ssrdev0", "/hd:0/SSR1", FS_BASED, true, true, false, " ", " ", 0 }, -{"/ssrdev1", "/hd:0/SSR2", FS_BASED, true, true, false, " ", " ", 0 }, -{"/ssrdev2", "/hd:0/SSR3", FS_BASED, true, true, false, " ", " ", 0 }, - -{"unused", "unused", FS_BASED, true, true, false, " ", " ", 0 }, -{"unused", "unused", FS_BASED, true, true, false, " ", " ", 0 }, -{"unused", "unused", FS_BASED, true, true, false, " ", " ", 0 }, -{"unused", "unused", FS_BASED, true, true, false, " ", " ", 0 } - -}; - - - diff --git a/src/bsp/pc-linux/CMakeLists.txt b/src/bsp/pc-linux/CMakeLists.txt index ba7d3ecb4..40b0aaca7 100644 --- a/src/bsp/pc-linux/CMakeLists.txt +++ b/src/bsp/pc-linux/CMakeLists.txt @@ -10,7 +10,6 @@ add_library(osal_pc-linux_impl OBJECT src/bsp_start.c - src/bsp_voltab.c src/bsp_console.c ) diff --git a/src/bsp/pc-linux/build_options.cmake b/src/bsp/pc-linux/build_options.cmake index c0a7a5c07..bf1d3c356 100644 --- a/src/bsp/pc-linux/build_options.cmake +++ b/src/bsp/pc-linux/build_options.cmake @@ -21,8 +21,3 @@ if (NOT CMAKE_CROSSCOMPILING) set(UT_COVERAGE_COMPILE_FLAGS -pg --coverage) set(UT_COVERAGE_LINK_FLAGS -pg --coverage) endif() - -# This indicates where to stage target binaries created during the build -# It should reflect the _real_ location of the persistent storage path used by -# the BSP which is intended to be used for runtime modules or files. -set(OSAL_BSP_STAGING_INSTALL_DIR "eeprom1") diff --git a/src/bsp/pc-linux/src/bsp_start.c b/src/bsp/pc-linux/src/bsp_start.c index 50d080fcb..964e0aba2 100644 --- a/src/bsp/pc-linux/src/bsp_start.c +++ b/src/bsp/pc-linux/src/bsp_start.c @@ -38,41 +38,9 @@ OS_BSP_PcLinuxGlobalData_t OS_BSP_PcLinuxGlobal; --------------------------------------------------------- */ void OS_BSP_Initialize(void) { - mode_t mode; - uint32 i; - struct stat statbuf; FILE *fp; char buffer[32]; - /* - ** Create local directories for "disk" mount points - ** See bsp_voltab for the values - ** - ** NOTE - the voltab table is poorly designed here; values of "0" are valid - ** and will translate into an entry that is actually used. In particular the - ** "free" flag has to be actually initialized to TRUE to say its NOT valid. - ** So in the case of an entry that has been zeroed out (i.e. bss section) it - ** will be treated as a valid entry. - ** - ** Checking that the DeviceName starts with a leading slash '/' is a workaround - ** for this, and may be the only way to detect an entry that is uninitialized. - */ - mode = S_IFDIR | S_IRWXU | S_IRWXG | S_IRWXO; - for (i = 0; i < NUM_TABLE_ENTRIES; ++i) - { - if (OS_VolumeTable[i].VolumeType == FS_BASED && - OS_VolumeTable[i].PhysDevName[0] != 0 && - OS_VolumeTable[i].DeviceName[0] == '/') - - { - if (stat(OS_VolumeTable[i].PhysDevName, &statbuf) < 0) - { - BSP_DEBUG("Creating mount point: %s\n", OS_VolumeTable[i].PhysDevName); - mkdir(OS_VolumeTable[i].PhysDevName, mode); - } - } - } - /* * If not running as root, check /proc/sys/fs/mqueue/msg_max * diff --git a/src/bsp/pc-linux/src/bsp_voltab.c b/src/bsp/pc-linux/src/bsp_voltab.c deleted file mode 100644 index ca881b265..000000000 --- a/src/bsp/pc-linux/src/bsp_voltab.c +++ /dev/null @@ -1,49 +0,0 @@ -/* -** File : bsp_voltab.c -** -** This is governed by the NASA Open Source Agreement and may be used, -** distributed and modified only pursuant to the terms of that agreement. -** -** Copyright (c) 2004-2006, United States government as represented by the -** administrator of the National Aeronautics Space Administration. -** All rights reserved. -** -** -** Author : Nicholas Yanchik / GSFC Code 582 -** -** BSP Volume table for file systems -*/ - -/**************************************************************************************** - INCLUDE FILES -****************************************************************************************/ - -#include "pclinux_bsp_internal.h" - -/* -** volume table. -*/ -OS_VolumeInfo_t OS_VolumeTable [NUM_TABLE_ENTRIES] = -{ -/* Dev Name Phys Dev Vol Type Volatile? Free? IsMounted? Volname MountPt BlockSz */ -{"/ramdev0", "./ram0", FS_BASED, true, true, false, " ", " ", 0 }, -{"/ramdev1", "./ram1", FS_BASED, true, true, false, " ", " ", 0 }, -{"/ramdev2", "./ram2", FS_BASED, true, true, false, " ", " ", 0 }, -{"/ramdev3", "./ram3", FS_BASED, true, true, false, " ", " ", 0 }, -{"/ramdev4", "./ram4", FS_BASED, true, true, false, " ", " ", 0 }, -{"/ramdev5", "./ram5", FS_BASED, true, true, false, " ", " ", 0 }, - -/* -** The following entry is a "pre-mounted" path to a non-volatile device -*/ -{"/eedev0", "./eeprom1", FS_BASED, false, false, true, "CF", "/cf", 512 }, - -{"unused", "unused", FS_BASED, true, true, false, " ", " ", 0 }, -{"unused", "unused", FS_BASED, true, true, false, " ", " ", 0 }, -{"unused", "unused", FS_BASED, true, true, false, " ", " ", 0 }, -{"unused", "unused", FS_BASED, true, true, false, " ", " ", 0 }, -{"unused", "unused", FS_BASED, true, true, false, " ", " ", 0 }, -{"unused", "unused", FS_BASED, true, true, false, " ", " ", 0 }, -{"unused", "unused", FS_BASED, true, true, false, " ", " ", 0 } -}; - diff --git a/src/bsp/pc-rtems/CMakeLists.txt b/src/bsp/pc-rtems/CMakeLists.txt index b5df36a6c..9b5a0af99 100644 --- a/src/bsp/pc-rtems/CMakeLists.txt +++ b/src/bsp/pc-rtems/CMakeLists.txt @@ -6,7 +6,6 @@ add_library(osal_pc-rtems_impl OBJECT src/bsp_start.c - src/bsp_voltab.c src/bsp_console.c ) diff --git a/src/bsp/pc-rtems/build_options.cmake b/src/bsp/pc-rtems/build_options.cmake index 1690376ff..d32e9245a 100644 --- a/src/bsp/pc-rtems/build_options.cmake +++ b/src/bsp/pc-rtems/build_options.cmake @@ -8,8 +8,3 @@ target_link_libraries(osal_bsp rtemscpu ) - -# This indicates where to stage target binaries created during the build -# It should reflect the _real_ location of the persistent storage path used by -# the BSP which is intended to be used for runtime modules or files. -set(OSAL_BSP_STAGING_INSTALL_DIR "eeprom") diff --git a/src/bsp/pc-rtems/src/bsp_start.c b/src/bsp/pc-rtems/src/bsp_start.c index bcce3f6e2..f9dbe2f3e 100644 --- a/src/bsp/pc-rtems/src/bsp_start.c +++ b/src/bsp/pc-rtems/src/bsp_start.c @@ -101,9 +101,7 @@ OS_BSP_PcRtemsGlobalData_t OS_BSP_PcRtemsGlobal; void OS_BSP_Setup(void) { int status; - unsigned int i; struct stat statbuf; - const char * cfpart; const char * cmdlinestr; const char * cmdp; char * cmdi, *cmdo; @@ -206,6 +204,19 @@ void OS_BSP_Setup(void) printf("Creating Root file system failed: %s\n", rtems_status_text(status)); } + /* + * Create the mountpoint for the general purpose file system + */ + if (stat(RTEMS_USER_FS_MOUNTPOINT, &statbuf) < 0) + { + status = mkdir(RTEMS_USER_FS_MOUNTPOINT, + S_IFDIR | S_IRWXU | S_IRWXG | S_IRWXO); /* For nonvol filesystem mountpoint */ + if (status < 0) + { + printf("mkdir failed: %s\n", strerror(errno)); + } + } + /* * Register the IDE partition table. */ @@ -217,52 +228,25 @@ void OS_BSP_Setup(void) * is still available. */ BSP_DEBUG("warning: /dev/hda partition table not found: %s / %s\n", rtems_status_text(status), strerror(errno)); BSP_DEBUG("Persistent storage will NOT be mounted\n"); - cfpart = NULL; } else { - cfpart = "/dev/hda1"; + status = mount("/dev/hda1", RTEMS_USER_FS_MOUNTPOINT, RTEMS_FILESYSTEM_TYPE_DOSFS, + RTEMS_FILESYSTEM_READ_WRITE, NULL); + if (status < 0) + { + BSP_DEBUG("mount failed: %s\n", strerror(errno)); + } } /* - ** Create local directories for "disk" mount points - ** See bsp_voltab for the values - ** - ** NOTE - the voltab table is poorly designed here; values of "0" are valid - ** and will translate into an entry that is actually used. In particular the - ** "free" flag has to be actually initialized to TRUE to say its NOT valid. - ** So in the case of an entry that has been zeroed out (i.e. bss section) it - ** will be treated as a valid entry. - ** - ** Checking that the DeviceName starts with a leading slash '/' is a workaround - ** for this, and may be the only way to detect an entry that is uninitialized. - */ - for (i = 0; i < NUM_TABLE_ENTRIES; ++i) - { - if (OS_VolumeTable[i].VolumeType == FS_BASED && OS_VolumeTable[i].PhysDevName[0] != 0 && - OS_VolumeTable[i].DeviceName[0] == '/') + * Change to the user storage mountpoint dir, which + * will be the basis of relative directory refs. + * If mounted, it will be persistent, otherwise + * it will be an IMFS dir, but should generally work. + */ + chdir(RTEMS_USER_FS_MOUNTPOINT); - { - if (stat(OS_VolumeTable[i].PhysDevName, &statbuf) < 0) - { - status = mkdir(OS_VolumeTable[i].PhysDevName, - S_IFDIR | S_IRWXU | S_IRWXG | S_IRWXO); /* For ramdisk mountpoint */ - if (status < 0) - { - printf("mkdir failed: %s\n", strerror(errno)); - } - } - if (cfpart != NULL && strcmp(OS_VolumeTable[i].MountPoint, "/cf") == 0) - { - status = mount(cfpart, OS_VolumeTable[i].PhysDevName, RTEMS_FILESYSTEM_TYPE_DOSFS, - RTEMS_FILESYSTEM_READ_WRITE, NULL); - if (status < 0) - { - printf("mount failed: %s\n", strerror(errno)); - } - } - } - } /* * Start the shell now, before any application starts. diff --git a/src/bsp/pc-rtems/src/bsp_voltab.c b/src/bsp/pc-rtems/src/bsp_voltab.c deleted file mode 100644 index f927d7abc..000000000 --- a/src/bsp/pc-rtems/src/bsp_voltab.c +++ /dev/null @@ -1,52 +0,0 @@ -/* -** File : bsp_voltab.c -** -** This is governed by the NASA Open Source Agreement and may be used, -** distributed and modified only pursuant to the terms of that agreement. -** -** Copyright (c) 2004-2006, United States government as represented by the -** administrator of the National Aeronautics Space Administration. -** All rights reserved. -** -** -** BSP Volume table for file systems -*/ - -/**************************************************************************************** - INCLUDE FILES -****************************************************************************************/ -#include "common_types.h" -#include "osapi.h" - -/* -** volume table. This table has the OS_ name, since it belongs to the OSAL, not the CFE_PSP -*/ -OS_VolumeInfo_t OS_VolumeTable [NUM_TABLE_ENTRIES] = -{ -/* Dev Name Phys Dev Vol Type Volatile? Free? IsMounted? Volname MountPt BlockSz */ - -/* cFE RAM Disk */ -{ "/ramdev0", "/ram0", FS_BASED, true, true, false, " ", " ", 512 }, - -/* cFE non-volatile Disk -- Auto-Mapped to an existing CF disk */ -{"/eedev0", "/eeprom", FS_BASED, false, false, true, "CF", "/cf", 512 }, - -{"unused", "unused", FS_BASED, true, true, false, " ", " ", 0 }, -{"unused", "unused", FS_BASED, true, true, false, " ", " ", 0 }, -{"unused", "unused", FS_BASED, true, true, false, " ", " ", 0 }, -{"unused", "unused", FS_BASED, true, true, false, " ", " ", 0 }, -{"unused", "unused", FS_BASED, true, true, false, " ", " ", 0 }, - -{"unused", "unused", FS_BASED, true, true, false, " ", " ", 0 }, -{"unused", "unused", FS_BASED, true, true, false, " ", " ", 0 }, -{"unused", "unused", FS_BASED, true, true, false, " ", " ", 0 }, - -{"unused", "unused", FS_BASED, true, true, false, " ", " ", 0 }, -{"unused", "unused", FS_BASED, true, true, false, " ", " ", 0 }, -{"unused", "unused", FS_BASED, true, true, false, " ", " ", 0 }, -{"unused", "unused", FS_BASED, true, true, false, " ", " ", 0 } - -}; - - - diff --git a/src/bsp/pc-rtems/src/pcrtems_bsp_internal.h b/src/bsp/pc-rtems/src/pcrtems_bsp_internal.h index 89656d2c7..0903dc945 100644 --- a/src/bsp/pc-rtems/src/pcrtems_bsp_internal.h +++ b/src/bsp/pc-rtems/src/pcrtems_bsp_internal.h @@ -31,6 +31,11 @@ #define RTEMS_NUMBER_OF_RAMDISKS 1 #define RTEMS_MAX_CMDLINE 256 +/* + * The location which the general purpose file system will be mounted + */ +#define RTEMS_USER_FS_MOUNTPOINT "/mnt" + /* * By default put the shell at the same priority * as the utility task which handles OS_printf() diff --git a/src/bsp/shared/inc/bsp-impl.h b/src/bsp/shared/inc/bsp-impl.h index bce51a9bb..3534fbad5 100644 --- a/src/bsp/shared/inc/bsp-impl.h +++ b/src/bsp/shared/inc/bsp-impl.h @@ -91,10 +91,13 @@ typedef struct */ extern OS_BSP_GlobalData_t OS_BSP_Global; +#ifndef OSAL_OMIT_DEPRECATED /* * Volume Table declaration (supplied by BSP; typically defined in bsp_voltab.c) + * @deprecated Use OS File System API to register volumes. */ -extern OS_VolumeInfo_t OS_VolumeTable[NUM_TABLE_ENTRIES]; +extern OS_VolumeInfo_t OS_VolumeTable[OS_MAX_FILE_SYSTEMS]; +#endif /********************************************************************/ /* INTERNAL BSP IMPLEMENTATION FUNCTIONS */ diff --git a/src/bsp/shared/src/bsp_default_voltab.c b/src/bsp/shared/src/bsp_default_voltab.c new file mode 100644 index 000000000..4786585b4 --- /dev/null +++ b/src/bsp/shared/src/bsp_default_voltab.c @@ -0,0 +1,62 @@ +/* +** File : bsp_voltab.c +** +** This is governed by the NASA Open Source Agreement and may be used, +** distributed and modified only pursuant to the terms of that agreement. +** +** Copyright (c) 2004-2006, United States government as represented by the +** administrator of the National Aeronautics Space Administration. +** All rights reserved. +** +** +** BSP Volume table for file systems +** +** Define a default OS volume table, which has no valid entries in it. +** This is a deprecated structure in the process of being phased out. +** +** This source file should only be compiled and included in "osal_bsp" when +** OSAL_OMIT_DEPRECATED is false/unset. +** +** This serves to decouple the PSP changes and the OSAL_OMIT_DEPRECATED setting - +** allowing a PSP to be updated to use only the recommended API and remove the +** OS_VolumeTable, while still allowing code to build and link when OSAL_OMIT_DEPRECATED +** is not set. +** +** If a classic PSP is still providing this symbol, then the PSP-provided symbol will +** be used, and this one will be ignored, preserving old behavior. +** +** If an updated PSP has removed this symbol, then this will be used to satisfy linking +** requirements when building without OSAL_OMIT_DEPRECATED. +** +*/ + +/**************************************************************************************** + INCLUDE FILES +****************************************************************************************/ +#include "common_types.h" +#include "osapi.h" + +OS_VolumeInfo_t OS_VolumeTable [OS_MAX_FILE_SYSTEMS] = +{ + /* Dev Name Phys Dev Vol Type Volatile? Free? IsMounted? Volname MountPt BlockSz */ + {"unused", "unused", FS_BASED, true, true, false, " ", " ", 0 }, + {"unused", "unused", FS_BASED, true, true, false, " ", " ", 0 }, + {"unused", "unused", FS_BASED, true, true, false, " ", " ", 0 }, + {"unused", "unused", FS_BASED, true, true, false, " ", " ", 0 }, + {"unused", "unused", FS_BASED, true, true, false, " ", " ", 0 }, + {"unused", "unused", FS_BASED, true, true, false, " ", " ", 0 }, + {"unused", "unused", FS_BASED, true, true, false, " ", " ", 0 }, + + {"unused", "unused", FS_BASED, true, true, false, " ", " ", 0 }, + {"unused", "unused", FS_BASED, true, true, false, " ", " ", 0 }, + {"unused", "unused", FS_BASED, true, true, false, " ", " ", 0 }, + + {"unused", "unused", FS_BASED, true, true, false, " ", " ", 0 }, + {"unused", "unused", FS_BASED, true, true, false, " ", " ", 0 }, + {"unused", "unused", FS_BASED, true, true, false, " ", " ", 0 }, + {"unused", "unused", FS_BASED, true, true, false, " ", " ", 0 } + +}; + + + diff --git a/src/os/inc/osapi-os-filesys.h b/src/os/inc/osapi-os-filesys.h index 5739a66f1..d928c1cf3 100644 --- a/src/os/inc/osapi-os-filesys.h +++ b/src/os/inc/osapi-os-filesys.h @@ -38,20 +38,24 @@ #define OS_CHK_ONLY 0 /**< Unused, API takes bool */ #define OS_REPAIR 1 /**< Unused, API takes bool */ +#ifndef OSAL_OMIT_DEPRECATED + /** @defgroup OSVolType OSAL Volume Type Defines * @{ */ -#define FS_BASED 0 /**< Volume type FS based */ -#define RAM_DISK 1 /**< Volume type RAM disk */ -#define EEPROM_DISK 2 /**< Volume type EEPROM disk */ -#define ATA_DISK 3 /**< Volume type ATA disk */ +#define FS_BASED 0 /**< @deprecated Volume type FS based */ +#define RAM_DISK 1 /**< @deprecated Volume type RAM disk */ +#define EEPROM_DISK 2 /**< @deprecated Volume type EEPROM disk */ +#define ATA_DISK 3 /**< @deprecated Volume type ATA disk */ /**@}*/ /** * @brief Number of entries in the internal volume table + * @deprecated */ -#define NUM_TABLE_ENTRIES 14 +#define NUM_FILE_SYSTEMS OS_MAX_FILE_SYSTEMS +#endif /* ** Length of a Device and Volume name */ @@ -113,11 +117,12 @@ * os_err_name_t. */ typedef os_err_name_t os_fs_err_name_t; -#endif /** * @brief Internal structure of the OS volume table for * mounted file systems and path translation + * + * @deprecated Use the OSAL file system API to register volumes */ typedef struct { @@ -133,6 +138,9 @@ typedef struct } OS_VolumeInfo_t; +#endif + + /** @brief OSAL file system info */ typedef struct { diff --git a/src/os/shared/inc/os-shared-filesys.h b/src/os/shared/inc/os-shared-filesys.h index d578e57bf..2175316fd 100644 --- a/src/os/shared/inc/os-shared-filesys.h +++ b/src/os/shared/inc/os-shared-filesys.h @@ -199,11 +199,13 @@ int32 OS_FileSysUnmountVolume_Impl (uint32 filesys_id); */ bool OS_FileSys_FindVirtMountPoint(void *ref, uint32 local_id, const OS_common_record_t *obj); -int32 OS_FileSys_InitLocalFromVolTable(OS_filesys_internal_record_t *local, const OS_VolumeInfo_t *Vol); int32 OS_FileSys_SetupInitialParamsForDevice(const char *devname, OS_filesys_internal_record_t *local); int32 OS_FileSys_Initialize(char *address, const char *fsdevname, const char * fsvolname, uint32 blocksize, uint32 numblocks, bool should_format); +#ifndef OSAL_OMIT_DEPRECATED +int32 OS_FileSys_InitLocalFromVolTable(OS_filesys_internal_record_t *local, const OS_VolumeInfo_t *Vol); +#endif #endif /* INCLUDE_OS_SHARED_FILESYS_H_ */ diff --git a/src/os/shared/src/osapi-filesys.c b/src/os/shared/src/osapi-filesys.c index c2638ad70..ac98076f5 100644 --- a/src/os/shared/src/osapi-filesys.c +++ b/src/os/shared/src/osapi-filesys.c @@ -48,24 +48,18 @@ enum */ OS_filesys_internal_record_t OS_filesys_table[LOCAL_NUM_OBJECTS]; +#ifndef OSAL_OMIT_DEPRECATED -#ifndef OS_DISABLE_VOLUME_TABLE /* * This is the volume table reference. It is defined in the BSP/startup code for the board * In this implementation it is treated as a "const" -- any dynamic updates such as runtime * mount points are handled with an internal table. + * + * Use of the static volume table is deprecated. New applications should register the file + * system mappings via runtime API calls instead (e.g. OS_FileSysAddFixedMap). */ extern const OS_VolumeInfo_t OS_VolumeTable[]; -#define OS_COMPAT_VOLTAB OS_VolumeTable -#define OS_COMPAT_VOLTAB_SIZE NUM_TABLE_ENTRIES -#else -/* - * Alternatively, this module can work without an OS_VolumeTable at all. - * In this mode, the PSP/BSP can explicitly instantiate any filesystem mappings - * using the runtime API call(s) prior to starting the app. - */ -#define OS_COMPAT_VOLTAB NULL -#define OS_COMPAT_VOLTAB_SIZE 0 + #endif @@ -104,10 +98,14 @@ bool OS_FileSys_FindVirtMountPoint(void *ref, uint32 local_id, const OS_common_r * Purpose: Local helper routine, not part of OSAL API. * Pre-populates a local filesys table entry from the classic OS_VolumeTable * This provides backward compatibility with existing PSP/BSP implementations. + * + * This helper is not necessary when not using the OS_VolumeTable and therefore + * can be compiled-out when OSAL_OMIT_DEPRECATED is set. * * Returns: OS_SUCCESS on success or appropriate error code. * *-----------------------------------------------------------------*/ +#ifndef OSAL_OMIT_DEPRECATED int32 OS_FileSys_InitLocalFromVolTable(OS_filesys_internal_record_t *local, const OS_VolumeInfo_t *Vol) { int32 return_code = OS_SUCCESS; @@ -198,7 +196,7 @@ int32 OS_FileSys_InitLocalFromVolTable(OS_filesys_internal_record_t *local, cons return return_code; } /* end OS_FileSys_InitLocalFromVolTable */ - +#endif /* OSAL_OMIT_DEPRECATED */ /*---------------------------------------------------------------- * @@ -208,18 +206,21 @@ int32 OS_FileSys_InitLocalFromVolTable(OS_filesys_internal_record_t *local, cons * Pre-populates a local filesys table entry from the classic OS_VolumeTable * This provides backward compatibility with existing PSP/BSP implementations. * + * This function is a no-op when OSAL_OMIT_DEPRECATED is set. + * * Returns: OS_SUCCESS on success or appropriate error code. * *-----------------------------------------------------------------*/ int32 OS_FileSys_SetupInitialParamsForDevice(const char *devname, OS_filesys_internal_record_t *local) { + int32 return_code = OS_ERR_NAME_NOT_FOUND; + +#ifndef OSAL_OMIT_DEPRECATED const OS_VolumeInfo_t *Vol; - int32 return_code; uint32 i; - return_code = OS_ERR_NAME_NOT_FOUND; - Vol = OS_COMPAT_VOLTAB; - for (i = 0; i < OS_COMPAT_VOLTAB_SIZE; i++) + Vol = OS_VolumeTable; + for (i = 0; i < OS_MAX_FILE_SYSTEMS; i++) { if (strcmp(Vol->DeviceName, devname) == 0) { @@ -229,6 +230,7 @@ int32 OS_FileSys_SetupInitialParamsForDevice(const char *devname, OS_filesys_int ++Vol; } +#endif /* OSAL_OMIT_DEPRECATED */ return return_code; } /* end OS_FileSys_SetupInitialParamsForDevice */ @@ -360,15 +362,17 @@ int32 OS_FileSys_Initialize(char *address, const char *fsdevname, const char * f *-----------------------------------------------------------------*/ int32 OS_FileSysAPI_Init(void) { + int32 return_code = OS_SUCCESS; + + memset(OS_filesys_table, 0, sizeof(OS_filesys_table)); + +#ifndef OSAL_OMIT_DEPRECATED uint32 i; uint32 local_id; - int32 return_code = OS_SUCCESS; OS_common_record_t *global; OS_filesys_internal_record_t *local; const OS_VolumeInfo_t *Vol; - memset(OS_filesys_table, 0, sizeof(OS_filesys_table)); - /* * For compatibility, migrate active entries of the BSP-provided OS_VolumeTable * into the local filesystem table. In this implementation, the OS_VolumeTable @@ -392,8 +396,8 @@ int32 OS_FileSysAPI_Init(void) * Most existing PSP packages seem to set the device name in unused entries * to the special string "unused", whereas a valid entry starts with a slash (/). */ - Vol = OS_COMPAT_VOLTAB; - for (i = 0; i < OS_COMPAT_VOLTAB_SIZE && return_code == OS_SUCCESS; i++) + Vol = OS_VolumeTable; + for (i = 0; i < OS_MAX_FILE_SYSTEMS && return_code == OS_SUCCESS; i++) { if (Vol->DeviceName[0] == '/' && Vol->FreeFlag == false) { @@ -413,6 +417,7 @@ int32 OS_FileSysAPI_Init(void) } ++Vol; } +#endif return return_code; } /* end OS_FileSysAPI_Init */ @@ -1026,7 +1031,7 @@ int32 OS_GetFsInfo(os_fsinfo_t *filesys_info) OS_Lock_Global_Impl(OS_OBJECT_TYPE_OS_FILESYS); - for ( i = 0; i < NUM_TABLE_ENTRIES; i++ ) + for ( i = 0; i < OS_MAX_FILE_SYSTEMS; i++ ) { if ( OS_global_filesys_table[i].active_id == 0) { diff --git a/src/unit-test-coverage/shared/src/coveragetest-filesys.c b/src/unit-test-coverage/shared/src/coveragetest-filesys.c index e84d14ec7..aa6d64de4 100644 --- a/src/unit-test-coverage/shared/src/coveragetest-filesys.c +++ b/src/unit-test-coverage/shared/src/coveragetest-filesys.c @@ -551,51 +551,6 @@ void Test_OS_FileSys_FindVirtMountPoint(void) UtAssert_True(result, "OS_FileSys_FindVirtMountPoint(%s) (nominal) == true", refstr); } -/* - * Specific volume table entries to exercise translation cases in OS_FileSys_InitLocalFromVolTable() - */ -static const OS_VolumeInfo_t UT_VOLTAB_TESTCASES[] = -{ - { .DeviceName = "/UT1", .VolumeType = ATA_DISK, .FreeFlag = false, .IsMounted = true }, - { .DeviceName = "/UT2", .VolumeType = EEPROM_DISK, .FreeFlag = true, .IsMounted = false }, - { .DeviceName = "/UT3", .VolumeType = RAM_DISK, .FreeFlag = true, .IsMounted = false } -}; - -void Test_OS_FileSys_InitLocalFromVolTable(void) -{ - /* - * Test Case For: - * static int32 OS_FileSys_InitLocalFromVolTable(OS_filesys_internal_record_t *local, const OS_VolumeInfo_t *Vol) - * - * This is a static internal function and must be invoked through a UT-specific wrapper in - * order to get coverage on it. - */ - OS_filesys_internal_record_t temprec; - int32 actual; - int32 expected; - - - /* this should return OS_ERROR because the mount point was not valid */ - memset(&temprec,0,sizeof(temprec)); - expected = OS_ERROR; - actual = OS_FileSys_InitLocalFromVolTable(&temprec, &UT_VOLTAB_TESTCASES[0]); - UtAssert_True(actual == expected, "OS_FileSys_InitLocalFromVolTable(0) (%ld) == OS_ERROR", (long)actual); - - memset(&temprec,0,sizeof(temprec)); - expected = OS_SUCCESS; - actual = OS_FileSys_InitLocalFromVolTable(&temprec, &UT_VOLTAB_TESTCASES[1]); - UtAssert_True(actual == expected, "OS_FileSys_InitLocalFromVolTable(1) (%ld) == OS_SUCCESS", (long)actual); - UtAssert_True(temprec.fstype == OS_FILESYS_TYPE_MTD, "OS_FileSys_InitLocalFromVolTable(1) fstype(%u) == OS_FILESYS_TYPE_MTD", - (unsigned int)temprec.fstype); - - memset(&temprec,0,sizeof(temprec)); - expected = OS_SUCCESS; - actual = OS_FileSys_InitLocalFromVolTable(&temprec, &UT_VOLTAB_TESTCASES[2]); - UtAssert_True(actual == expected, "OS_FileSys_InitLocalFromVolTable(1) (%ld) == OS_SUCCESS", (long)actual); - UtAssert_True(temprec.fstype == OS_FILESYS_TYPE_VOLATILE_DISK, "OS_FileSys_InitLocalFromVolTable(2) fstype(%u) == OS_FILESYS_TYPE_MTD", - (unsigned int)temprec.fstype); -} - /* Osapi_Test_Setup * * Purpose: @@ -637,7 +592,6 @@ void UtTest_Setup(void) ADD_TEST(OS_GetFsInfo); ADD_TEST(OS_TranslatePath); ADD_TEST(OS_FileSys_FindVirtMountPoint); - ADD_TEST(OS_FileSys_InitLocalFromVolTable); } diff --git a/src/unit-tests/osfile-test/ut_osfile_dirio_test.h b/src/unit-tests/osfile-test/ut_osfile_dirio_test.h index 8d689b909..d843f7cbb 100644 --- a/src/unit-tests/osfile-test/ut_osfile_dirio_test.h +++ b/src/unit-tests/osfile-test/ut_osfile_dirio_test.h @@ -17,7 +17,7 @@ ** Macros **--------------------------------------------------------------------------------*/ -#define UT_OS_FILE_LIST_LEN (NUM_TABLE_ENTRIES + 10) +#define UT_OS_FILE_LIST_LEN (OS_MAX_FILE_SYSTEMS + 10) /*--------------------------------------------------------------------------------* ** Data types diff --git a/src/unit-tests/osfilesys-test/ut_osfilesys_diskio_test.c b/src/unit-tests/osfilesys-test/ut_osfilesys_diskio_test.c index 7e804156a..3cd6f97ad 100644 --- a/src/unit-tests/osfilesys-test/ut_osfilesys_diskio_test.c +++ b/src/unit-tests/osfilesys-test/ut_osfilesys_diskio_test.c @@ -88,7 +88,7 @@ extern char g_mntNames[UT_OS_FILESYS_LIST_LEN][UT_OS_FILE_BUFF_SIZE]; ** (a) OS_FS_ERR_DRIVE_NOT_CREATED ** ----------------------------------------------------- ** Test #4: Disk-full condition -** 1) Call this routine (NUM_TABLE_ENTRIES+1) of times +** 1) Call this routine (OS_MAX_FILE_SYSTEMS+1) of times ** 2) Expect the returned value to be (except the last call) ** (a) OS_SUCCESS ** 3) Expect the returned value of the last call to be @@ -148,7 +148,7 @@ void UT_os_initfs_test() /*-----------------------------------------------------*/ testDesc = "#4 Disk-full"; - for (i=0; i <= NUM_TABLE_ENTRIES; i++) + for (i=0; i <= OS_MAX_FILE_SYSTEMS; i++) { memset(g_devNames[i], '\0', sizeof(g_devNames[i])); UT_os_sprintf(g_devNames[i], "/ramdev%d", (int)i); @@ -226,7 +226,7 @@ void UT_os_initfs_test() ** (a) OS_FS_ERR_DRIVE_NOT_CREATED ** ----------------------------------------------------- ** Test #4: Disk-full condition -** 1) Call this routine (NUM_TABLE_ENTRIES+1) of times +** 1) Call this routine (OS_MAX_FILE_SYSTEMS+1) of times ** 2) Expect the returned value to be (except the last call) ** (a) OS_SUCCESS ** 3) Expect the returned value of the last call to be @@ -284,7 +284,7 @@ void UT_os_makefs_test() /*-----------------------------------------------------*/ testDesc = "#4 Disk-full"; - for (i=0; i <= NUM_TABLE_ENTRIES; i++) + for (i=0; i <= OS_MAX_FILE_SYSTEMS; i++) { memset(g_devNames[i], '\0', sizeof(g_devNames[i])); UT_os_sprintf(g_devNames[i], "/ramdev%d", (int)i); diff --git a/src/unit-tests/osfilesys-test/ut_osfilesys_diskio_test.h b/src/unit-tests/osfilesys-test/ut_osfilesys_diskio_test.h index 4e190c075..b08228c65 100644 --- a/src/unit-tests/osfilesys-test/ut_osfilesys_diskio_test.h +++ b/src/unit-tests/osfilesys-test/ut_osfilesys_diskio_test.h @@ -17,7 +17,7 @@ ** Macros **--------------------------------------------------------------------------------*/ -#define UT_OS_FILESYS_LIST_LEN (NUM_TABLE_ENTRIES + 10) +#define UT_OS_FILESYS_LIST_LEN (OS_MAX_FILE_SYSTEMS + 10) /*--------------------------------------------------------------------------------* ** Data types diff --git a/src/ut-stubs/utstub-helpers.c b/src/ut-stubs/utstub-helpers.c index 11147cc53..3561c995a 100644 --- a/src/ut-stubs/utstub-helpers.c +++ b/src/ut-stubs/utstub-helpers.c @@ -36,7 +36,7 @@ const uint32 UT_MAXOBJS[UT_OBJTYPE_MAX] = [UT_OBJTYPE_MODULE] = OS_MAX_MODULES, [UT_OBJTYPE_FILESTREAM] = OS_MAX_NUM_OPEN_FILES, [UT_OBJTYPE_TIMEBASE] = OS_MAX_TIMEBASES, - [UT_OBJTYPE_FILESYS] = NUM_TABLE_ENTRIES, + [UT_OBJTYPE_FILESYS] = OS_MAX_FILE_SYSTEMS, [UT_OBJTYPE_DIR] = OS_MAX_NUM_OPEN_DIRS }; From b25146641ee3471e5f485efaea1f1a218c88a78f Mon Sep 17 00:00:00 2001 From: Joseph Hickey Date: Thu, 14 May 2020 08:05:37 -0400 Subject: [PATCH 2/4] Fix #460, UT dependencies on BSP volume maps Do not assume BSP volume table provides a "/cf" directory map. Instead, create a UT-specific map of a consistent name, and use that. --- osconfig.h.in | 7 ++++ src/tests/file-api-test/file-api-test.c | 19 +++++++-- src/unit-tests/osloader-test/CMakeLists.txt | 2 +- .../osloader-test/ut_osloader_module_test.c | 6 +-- .../osloader-test/ut_osloader_symtable_test.c | 39 ++++++++----------- .../osloader-test/ut_osloader_test.c | 13 +++++++ .../ut_osloader_test_platforms.h | 38 +++--------------- 7 files changed, 61 insertions(+), 63 deletions(-) diff --git a/osconfig.h.in b/osconfig.h.in index 24bee2592..0da988e5b 100644 --- a/osconfig.h.in +++ b/osconfig.h.in @@ -248,5 +248,12 @@ */ #define OS_MAX_CONSOLES 1 +/** + * \brief The system-specific file extension used on loadable module files + * + * Fixed value based on system selection, not user configurable. + */ +#define OS_MODULE_FILE_EXTENSION "@CMAKE_SHARED_LIBRARY_SUFFIX@" + #endif /* INCLUDE_OSCONFIG_H_ */ diff --git a/src/tests/file-api-test/file-api-test.c b/src/tests/file-api-test/file-api-test.c index 502292746..29ca9bb9c 100644 --- a/src/tests/file-api-test/file-api-test.c +++ b/src/tests/file-api-test/file-api-test.c @@ -27,6 +27,8 @@ os_err_name_t errname; void UtTest_Setup(void) { + uint32 fs_id; + errname[0] = 0; if (OS_API_Init() != OS_SUCCESS) @@ -34,6 +36,15 @@ void UtTest_Setup(void) UtAssert_Abort("OS_API_Init() failed"); } + /* + * This test case requires a fixed virtual dir for one test case. + * Just map /test to a dir of the same name, relative to current dir. + */ + if (OS_FileSysAddFixedMap(&fs_id, "./test", "/test") != OS_SUCCESS) + { + UtAssert_Abort("OS_FileSysAddFixedMap() failed"); + } + /* * Register the test setup and check routines in UT assert * @@ -667,13 +678,13 @@ void TestOpenReadCloseDir(void) /* Test case for bug #181 - make sure that a directory used as a mount point * is able to be opened. This should not require a trailing - * slash (i.e. /cf rather than /cf/) */ - status = OS_DirectoryOpen(&dirh, "/cf"); - UtAssert_True(status >= OS_SUCCESS, "OS_DirectoryOpen /cf Id=%u Rc=%d",(unsigned int)dirh,(int)status); + * slash (i.e. /test rather than /test/) */ + status = OS_DirectoryOpen(&dirh, "/test"); + UtAssert_True(status >= OS_SUCCESS, "OS_DirectoryOpen /test Id=%u Rc=%d",(unsigned int)dirh,(int)status); /*Close the file */ status = OS_DirectoryClose(dirh); - UtAssert_True(status >= OS_SUCCESS, "OS_DirectoryClose /cf Rc=%d",(int)status); + UtAssert_True(status >= OS_SUCCESS, "OS_DirectoryClose /test Rc=%d",(int)status); /* remove the files */ status = OS_remove(filename1); diff --git a/src/unit-tests/osloader-test/CMakeLists.txt b/src/unit-tests/osloader-test/CMakeLists.txt index 349a274a5..72152120b 100644 --- a/src/unit-tests/osloader-test/CMakeLists.txt +++ b/src/unit-tests/osloader-test/CMakeLists.txt @@ -17,6 +17,6 @@ while(MOD GREATER 0) set_target_properties(MODULE${MOD} PROPERTIES COMPILE_DEFINITIONS "MODULE_NAME=module${MOD}" PREFIX "" - LIBRARY_OUTPUT_DIRECTORY eeprom1) + LIBRARY_OUTPUT_DIRECTORY utmod) add_dependencies(osal_loader_UT MODULE${MOD}) endwhile(MOD GREATER 0) diff --git a/src/unit-tests/osloader-test/ut_osloader_module_test.c b/src/unit-tests/osloader-test/ut_osloader_module_test.c index 7cde3d814..89dadbe8b 100644 --- a/src/unit-tests/osloader-test/ut_osloader_module_test.c +++ b/src/unit-tests/osloader-test/ut_osloader_module_test.c @@ -99,10 +99,8 @@ void UT_os_module_load_test() /* Setup */ for ( i = 0; i< OS_MAX_MODULES; i++ ) { - memset(module_name, '\0', sizeof(module_name)); - UT_os_sprintf(module_name, "MODULE%d",i); - memset(module_file_name, '\0', sizeof(module_file_name)); - UT_os_sprintf(module_file_name, UT_OS_SPECIFIC_MODULE_NAME, i); + snprintf(module_name, sizeof(module_name), UT_OS_GENERIC_MODULE_NAME_TEMPLATE, i); + snprintf(module_file_name, sizeof(module_file_name), UT_OS_GENERIC_MODULE_FILE_TEMPLATE, i); res = OS_ModuleLoad(&module_id, module_name, module_file_name); if ( res != OS_SUCCESS ) { diff --git a/src/unit-tests/osloader-test/ut_osloader_symtable_test.c b/src/unit-tests/osloader-test/ut_osloader_symtable_test.c index 04ce252f8..0f43dcd3a 100644 --- a/src/unit-tests/osloader-test/ut_osloader_symtable_test.c +++ b/src/unit-tests/osloader-test/ut_osloader_symtable_test.c @@ -128,15 +128,11 @@ void UT_os_symbol_table_dump_test() int32 res = 0; const char* testDesc; - /*-----------------------------------------------------*/ - testDesc = "API Not implemented"; - - res = OS_SymbolTableDump("/cf/apps/SymbolFile.dat", 32000); - if (res == OS_ERR_NOT_IMPLEMENTED) - { - UT_OS_TEST_RESULT( testDesc, UTASSERT_CASETYPE_NA); - goto UT_os_symbol_table_dump_test_exit_tag; - } + /* + * Note that even if the functionality is not implemented, + * the API still validates the input pointers (not null) and + * the validity of the file name. + */ /*-----------------------------------------------------*/ testDesc = "#1 Invalid-pointer-arg"; @@ -157,23 +153,22 @@ void UT_os_symbol_table_dump_test() UT_OS_TEST_RESULT( testDesc, UTASSERT_CASETYPE_FAILURE); /*-----------------------------------------------------*/ - testDesc = "#3 OS-call-failure"; - - UT_OS_TEST_RESULT( testDesc, UTASSERT_CASETYPE_INFO); - - /*-----------------------------------------------------*/ - testDesc = "#4 Nominal"; + testDesc = "#3 Nominal"; - /* Setup */ - res = OS_SymbolTableDump("/cf/apps/SymbolFile.dat", 32000); - if ( res == OS_SUCCESS ) + res = OS_SymbolTableDump(UT_OS_GENERIC_MODULE_DIR "SymbolFile.dat", 32000); + if (res == OS_ERR_NOT_IMPLEMENTED) + { + /* allowed, not applicable on this system */ + UT_OS_TEST_RESULT( testDesc, UTASSERT_CASETYPE_NA); + } + else if ( res == OS_SUCCESS ) + { UT_OS_TEST_RESULT( testDesc, UTASSERT_CASETYPE_PASS); + } else + { UT_OS_TEST_RESULT( testDesc, UTASSERT_CASETYPE_FAILURE); - -UT_os_symbol_table_dump_test_exit_tag: - return; - + } } /*================================================================================* diff --git a/src/unit-tests/osloader-test/ut_osloader_test.c b/src/unit-tests/osloader-test/ut_osloader_test.c index 24d12eb51..7dd6fc9f5 100644 --- a/src/unit-tests/osloader-test/ut_osloader_test.c +++ b/src/unit-tests/osloader-test/ut_osloader_test.c @@ -40,11 +40,24 @@ void UtTest_Setup(void) { + uint32 fs_id; + if (OS_API_Init() != OS_SUCCESS) { UtAssert_Abort("OS_API_Init() failed"); } + /* + * This test needs to load the modules from the filesystem, so + * there must be a virtual path corresponding to the path where + * the module files reside. This UT-specific mapping should be + * independent of the volume tables provided by the BSP. + */ + if (OS_FileSysAddFixedMap(&fs_id, "./utmod", "/utmod") != OS_SUCCESS) + { + UtAssert_Abort("OS_FileSysAddFixedMap() failed"); + } + UtTest_Add(UT_os_module_load_test, NULL, NULL, "OS_ModuleLoad"); UtTest_Add(UT_os_module_unload_test, NULL, NULL, "OS_ModuleUnload"); UtTest_Add(UT_os_module_info_test, NULL, NULL, "OS_ModuleInfo"); diff --git a/src/unit-tests/osloader-test/ut_osloader_test_platforms.h b/src/unit-tests/osloader-test/ut_osloader_test_platforms.h index fe17b76b3..a1fee6f96 100644 --- a/src/unit-tests/osloader-test/ut_osloader_test_platforms.h +++ b/src/unit-tests/osloader-test/ut_osloader_test_platforms.h @@ -15,40 +15,14 @@ ** Macros **--------------------------------------------------------------------------------*/ -/* - * The actual module files that the loader tests attempt to load need - * to be consistent with the system type that is being compiled for. - * - * It can be assumed that the BSP will provide some sort of virtual - * filesystem mapping for the /cf directory, but the file extension - * for a loadable module still differs. - */ +#define UT_OS_GENERIC_MODULE_DIR "/utmod/" +#define UT_OS_GENERIC_MODULE_BASENAME "MODULE" -/*--------------------------------------------*/ -#if defined(_VXWORKS_OS_) || defined(OSP_ARINC653) -/*--------------------------------------------*/ +#define UT_OS_GENERIC_MODULE_NAME1 UT_OS_GENERIC_MODULE_DIR UT_OS_GENERIC_MODULE_BASENAME "0" OS_MODULE_FILE_EXTENSION +#define UT_OS_GENERIC_MODULE_NAME2 UT_OS_GENERIC_MODULE_DIR UT_OS_GENERIC_MODULE_BASENAME "1" OS_MODULE_FILE_EXTENSION -#define UT_OS_GENERIC_MODULE_NAME1 "/cf/apps/MODULE.o" -#define UT_OS_GENERIC_MODULE_NAME2 "/cf/apps/MODULE1.o" -#define UT_OS_SPECIFIC_MODULE_NAME "/cf/apps/MODULE%d.o" - -/*--------------------------------------------*/ -#elif defined(_RTEMS_OS_) -/*--------------------------------------------*/ - -#define UT_OS_GENERIC_MODULE_NAME1 "/cf/MODULE.obj" -#define UT_OS_GENERIC_MODULE_NAME2 "/cf/MODULE1.obj" -#define UT_OS_SPECIFIC_MODULE_NAME "/cf/MODULE%d.obj" - -/*--------------------------------------------*/ -#else /* For any other OS assume Linux/POSIX style .so files */ -/*--------------------------------------------*/ - -#define UT_OS_GENERIC_MODULE_NAME1 "/cf/MODULE.so" -#define UT_OS_GENERIC_MODULE_NAME2 "/cf/MODULE1.so" -#define UT_OS_SPECIFIC_MODULE_NAME "/cf/MODULE%d.so" - -#endif +#define UT_OS_GENERIC_MODULE_NAME_TEMPLATE UT_OS_GENERIC_MODULE_BASENAME "%d" +#define UT_OS_GENERIC_MODULE_FILE_TEMPLATE UT_OS_GENERIC_MODULE_DIR UT_OS_GENERIC_MODULE_NAME_TEMPLATE OS_MODULE_FILE_EXTENSION /*--------------------------------------------------------------------------------* ** Data types From b51f5b683757deb339b104d2f081c09b2c0a43bd Mon Sep 17 00:00:00 2001 From: Joseph Hickey Date: Wed, 13 May 2020 20:12:45 -0400 Subject: [PATCH 3/4] Fix #456, improve filesystem type identification Add a distinct type for FS_BASED entries, and an UNKNOWN value for when a type is actually not yet identified. This can be used to identify the FS type first at the shared layer, then in the implementation layer as a fallback option if not identifiable in the shared layer. Use the volume name prefix "RAM" as a hint that the volume is supposed to be a VOLATILE disk as opposed to a normal disk. This is done in shared layer so it applies to all OS types. --- src/os/inc/osapi-os-filesys.h | 20 +++-- src/os/posix/src/os-impl-filesys.c | 23 ++++- src/os/rtems/src/os-impl-filesys.c | 60 +++++++++---- src/os/shared/inc/os-shared-filesys.h | 3 +- src/os/shared/src/osapi-filesys.c | 87 ++++++++++++------- src/os/vxworks/src/os-impl-filesys.c | 67 +++++++++++--- .../shared/src/coveragetest-filesys.c | 24 ++--- .../vxworks/src/coveragetest-filesys.c | 19 +++- 8 files changed, 213 insertions(+), 90 deletions(-) diff --git a/src/os/inc/osapi-os-filesys.h b/src/os/inc/osapi-os-filesys.h index d928c1cf3..530a83907 100644 --- a/src/os/inc/osapi-os-filesys.h +++ b/src/os/inc/osapi-os-filesys.h @@ -763,7 +763,7 @@ int32 OS_rmdir (const char *path); * @brief Create a fixed mapping between an existing directory and a virtual OSAL mount point. * * 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 OSAL. + * at runtime. It is intended to be called by the PSP/BSP prior to starting the application. * * @param[out] filesys_id An OSAL ID reflecting the file system * @param[in] phys_path The native system directory (an existing mount point) @@ -781,10 +781,15 @@ int32 OS_FileSysAddFixedMap(uint32 *filesys_id, const char *phys_path, * Makes a file system on the target. Highly dependent on underlying OS and * dependent on OS volume table definition. * + * @note The "volname" parameter of RAM disks should always begin with the string "RAM", + * e.g. "RAMDISK" or "RAM0","RAM1", etc if multiple devices are created. The underlying + * implementation uses this to select the correct filesystem type/format, and this may + * also be used to differentiate between RAM disks and real physical disks. + * * @param[in] address The address at which to start the new disk. If address == 0 * space will be allocated by the OS. - * @param[in] devname The name of the "generic" drive - * @param[in] volname The name of the volume (if needed, used on VxWorks) + * @param[in] devname The underlying kernel device to use, if applicable. + * @param[in] volname The name of the volume (see note) * @param[in] blocksize The size of a single block on the drive * @param[in] numblocks The number of blocks to allocate for the drive * @@ -816,10 +821,15 @@ int32 OS_mount (const char *devname, const char *mountpoint); * * Initializes a file system on the target. * + * @note The "volname" parameter of RAM disks should always begin with the string "RAM", + * e.g. "RAMDISK" or "RAM0","RAM1", etc if multiple devices are created. The underlying + * implementation uses this to select the correct filesystem type/format, and this may + * also be used to differentiate between RAM disks and real physical disks. + * * @param[in] address The address at which to start the new disk. If address == 0, * then space will be allocated by the OS - * @param[in] devname The name of the "generic" drive - * @param[in] volname The name of the volume (if needed, used on VxWorks) + * @param[in] devname The underlying kernel device to use, if applicable. + * @param[in] volname The name of the volume (see note) * @param[in] blocksize The size of a single block on the drive * @param[in] numblocks The number of blocks to allocate for the drive * diff --git a/src/os/posix/src/os-impl-filesys.c b/src/os/posix/src/os-impl-filesys.c index 3f09918fb..07a2b662f 100644 --- a/src/os/posix/src/os-impl-filesys.c +++ b/src/os/posix/src/os-impl-filesys.c @@ -47,6 +47,7 @@ /**************************************************************************************** GLOBAL DATA ***************************************************************************************/ +const char OS_POSIX_DEVICEFILE_PREFIX[] = "/dev/"; /**************************************************************************************** Filesys API @@ -93,13 +94,26 @@ int32 OS_FileSysStartVolume_Impl (uint32 filesys_id) VOLATILE_DISK_LOC_MAX }; + /* + * Determine basic type of filesystem, if not already known + */ + if (local->fstype == OS_FILESYS_TYPE_UNKNOWN && + strncmp(local->device_name, OS_POSIX_DEVICEFILE_PREFIX, sizeof(OS_POSIX_DEVICEFILE_PREFIX)-1) == 0) + { + /* + * If referring to a real device in the /dev filesystem, + * then assume it is a normal disk. + */ + local->fstype = OS_FILESYS_TYPE_NORMAL_DISK; + } /* - * For RAM_DISK volumes, there are two options: + * For VOLATILE volumes, there are two options: * - The /dev/shm filesystem, if it exists * - The /tmp filesystem * - * The /dev/shm is preferable because it should actually be a ramdisk. + * The /dev/shm is preferable because it should actually be a ramdisk, but + * it is system-specific - should exist on Linux if it is mounted. * The /tmp file system might be a regular persistent disk, but should always exist * on any POSIX-compliant OS. */ @@ -252,9 +266,10 @@ int32 OS_FileSysMountVolume_Impl (uint32 filesys_id) * mount point exists. For any other FS type, trigger an * error to indicate that it is not implemented in this OSAL. */ - if (local->fstype != OS_FILESYS_TYPE_VOLATILE_DISK) + if (local->fstype != OS_FILESYS_TYPE_VOLATILE_DISK && + local->fstype != OS_FILESYS_TYPE_FS_BASED) { - /* the mount command is only allowed on a ram disk */ + /* the mount command is not implemented for this FS type */ return OS_ERR_NOT_IMPLEMENTED; } diff --git a/src/os/rtems/src/os-impl-filesys.c b/src/os/rtems/src/os-impl-filesys.c index 40d127522..22affe70b 100644 --- a/src/os/rtems/src/os-impl-filesys.c +++ b/src/os/rtems/src/os-impl-filesys.c @@ -59,6 +59,11 @@ typedef struct GLOBAL DATA ***************************************************************************************/ +/* + * The prefix used for "real" device nodes on this platform + */ +const char OS_RTEMS_DEVICEFILE_PREFIX[] = "/dev/"; + /* * The implementation-specific file system state table. * This keeps record of the RTEMS driver and mount options for each filesystem @@ -116,12 +121,25 @@ int32 OS_FileSysStartVolume_Impl (uint32 filesys_id) return_code = OS_ERR_NOT_IMPLEMENTED; memset(impl,0,sizeof(*impl)); + /* + * Determine basic type of filesystem, if not already known + */ + if (local->fstype == OS_FILESYS_TYPE_UNKNOWN && + strncmp(local->device_name, OS_RTEMS_DEVICEFILE_PREFIX, sizeof(OS_RTEMS_DEVICEFILE_PREFIX)-1) == 0) + { + /* + * If referring to a real device in the /dev filesystem, + * then assume it is a normal disk. + */ + local->fstype = OS_FILESYS_TYPE_NORMAL_DISK; + } + /* * Take action based on the type of volume */ switch (local->fstype) { - case OS_FILESYS_TYPE_DEFAULT: + case OS_FILESYS_TYPE_FS_BASED: { /* * This "mount" type is basically not a mount at all, @@ -244,7 +262,7 @@ int32 OS_FileSysFormatVolume_Impl (uint32 filesys_id) switch(local->fstype) { - case OS_FILESYS_TYPE_DEFAULT: + case OS_FILESYS_TYPE_FS_BASED: { /* * In this mode a format is a no-op, as it is simply a directory @@ -323,14 +341,22 @@ int32 OS_FileSysMountVolume_Impl (uint32 filesys_id) } /* - ** Mount the Disk - */ - if ( mount(impl->blockdev_name, local->system_mountpt, - impl->mount_fstype, impl->mount_options, impl->mount_data) != 0 ) + * Only do the mount() syscall for real devices. + * For other types of filesystem mounts (e.g. FS_BASED), this is a no-op + */ + if (local->fstype == OS_FILESYS_TYPE_VOLATILE_DISK || + local->fstype == OS_FILESYS_TYPE_NORMAL_DISK) { - OS_DEBUG("OSAL: Error: mount of %s to %s failed: %s\n", - impl->blockdev_name, local->system_mountpt, strerror(errno)); - return OS_ERROR; + /* + ** Mount the Disk + */ + if ( mount(impl->blockdev_name, local->system_mountpt, + impl->mount_fstype, impl->mount_options, impl->mount_data) != 0 ) + { + OS_DEBUG("OSAL: Error: mount of %s to %s failed: %s\n", + impl->blockdev_name, local->system_mountpt, strerror(errno)); + return OS_ERROR; + } } return OS_SUCCESS; @@ -350,13 +376,17 @@ int32 OS_FileSysUnmountVolume_Impl (uint32 filesys_id) { OS_filesys_internal_record_t *local = &OS_filesys_table[filesys_id]; - /* - ** Try to unmount the disk - */ - if ( unmount(local->system_mountpt) < 0) + if (local->fstype == OS_FILESYS_TYPE_VOLATILE_DISK || + local->fstype == OS_FILESYS_TYPE_NORMAL_DISK) { - OS_DEBUG("OSAL: RTEMS unmount of %s failed :%s\n",local->system_mountpt, strerror(errno)); - return OS_ERROR; + /* + ** Try to unmount the disk + */ + if ( unmount(local->system_mountpt) < 0) + { + OS_DEBUG("OSAL: RTEMS unmount of %s failed :%s\n",local->system_mountpt, strerror(errno)); + return OS_ERROR; + } } return OS_SUCCESS; diff --git a/src/os/shared/inc/os-shared-filesys.h b/src/os/shared/inc/os-shared-filesys.h index 2175316fd..a7cd6918e 100644 --- a/src/os/shared/inc/os-shared-filesys.h +++ b/src/os/shared/inc/os-shared-filesys.h @@ -69,7 +69,8 @@ */ enum { - OS_FILESYS_TYPE_DEFAULT = 0, /**< Unspecified or unknown file system type */ + OS_FILESYS_TYPE_UNKNOWN = 0, /**< Unspecified or unknown file system type */ + OS_FILESYS_TYPE_FS_BASED, /**< A emulated virtual file system that maps to another file system location */ OS_FILESYS_TYPE_NORMAL_DISK, /**< A traditional disk drive or something that emulates one */ OS_FILESYS_TYPE_VOLATILE_DISK, /**< A temporary/volatile file system or RAM disk */ OS_FILESYS_TYPE_MTD, /**< A "memory technology device" such as FLASH or EEPROM */ diff --git a/src/os/shared/src/osapi-filesys.c b/src/os/shared/src/osapi-filesys.c index ac98076f5..f55224f97 100644 --- a/src/os/shared/src/osapi-filesys.c +++ b/src/os/shared/src/osapi-filesys.c @@ -62,6 +62,14 @@ extern const OS_VolumeInfo_t OS_VolumeTable[]; #endif +/* + * A string that should be the prefix of RAM disk volume names, which + * provides a hint that the file system refers to a RAM disk. + * + * If multiple RAM disks are required then these can be numbered, + * e.g. RAM0, RAM1, etc. + */ +const char OS_FILESYS_RAMDISK_VOLNAME_PREFIX[] = "RAM"; /*---------------------------------------------------------------- * @@ -146,6 +154,7 @@ int32 OS_FileSys_InitLocalFromVolTable(OS_filesys_internal_record_t *local, cons */ if (Vol->VolumeType == FS_BASED) { + local->fstype = OS_FILESYS_TYPE_FS_BASED; local->flags |= OS_FILESYS_FLAG_IS_FIXED; } else if (Vol->VolumeType == RAM_DISK || Vol->VolatileFlag) @@ -287,24 +296,7 @@ int32 OS_FileSys_Initialize(char *address, const char *fsdevname, const char * f /* Get the initial settings from the classic volume table. * If this fails, that is OK - because passed-in values get preference anyway */ - if (OS_FileSys_SetupInitialParamsForDevice(fsdevname, local) != OS_SUCCESS) - { - /* - * No known parameters for the device. - * - * if address was supplied, assume it is a RAM disk, otherwise - * assume it is a regular disk. - */ - if (address == NULL) - { - local->fstype = OS_FILESYS_TYPE_NORMAL_DISK; - } - else - { - local->fstype = OS_FILESYS_TYPE_VOLATILE_DISK; - } - - } + OS_FileSys_SetupInitialParamsForDevice(fsdevname, local); /* populate the VolumeName and BlockSize ahead of the Impl call, * so the implementation can reference this info if necessary */ @@ -313,6 +305,21 @@ int32 OS_FileSys_Initialize(char *address, const char *fsdevname, const char * f local->address = address; strcpy(local->volume_name, fsvolname); + /* + * Determine basic type of filesystem, if not already known + * + * if either an address was supplied, or if the volume name + * contains the string "RAM" then it is a RAM disk. Otherwise + * leave the type as UNKNOWN and let the implementation decide. + */ + if (local->fstype == OS_FILESYS_TYPE_UNKNOWN && + (local->address != NULL || + strncmp(local->volume_name, OS_FILESYS_RAMDISK_VOLNAME_PREFIX, + sizeof(OS_FILESYS_RAMDISK_VOLNAME_PREFIX)-1) == 0)) + { + local->fstype = OS_FILESYS_TYPE_VOLATILE_DISK; + } + return_code = OS_FileSysStartVolume_Impl(local_id); if (return_code == OS_SUCCESS) @@ -487,11 +494,30 @@ int32 OS_FileSysAddFixedMap(uint32 *filesys_id, const char *phys_path, const cha /* * mark the entry that it is a fixed disk */ - local->flags = - OS_FILESYS_FLAG_IS_FIXED | - OS_FILESYS_FLAG_IS_READY | - OS_FILESYS_FLAG_IS_MOUNTED_SYSTEM | - OS_FILESYS_FLAG_IS_MOUNTED_VIRTUAL; + local->fstype = OS_FILESYS_TYPE_FS_BASED; + local->flags = OS_FILESYS_FLAG_IS_FIXED; + + /* + * The "mount" implementation is required as it will + * create the mountpoint if it does not already exist + */ + return_code = OS_FileSysStartVolume_Impl(local_id); + + if (return_code == OS_SUCCESS) + { + local->flags |= OS_FILESYS_FLAG_IS_READY; + return_code = OS_FileSysMountVolume_Impl(local_id); + } + + if (return_code == OS_SUCCESS) + { + /* + * mark the entry that it is a fixed disk + */ + local->flags |= + OS_FILESYS_FLAG_IS_MOUNTED_SYSTEM | + OS_FILESYS_FLAG_IS_MOUNTED_VIRTUAL; + } /* Check result, finalize record, and unlock global table. */ return_code = OS_ObjectIdFinalizeNew(return_code, global, filesys_id); @@ -671,11 +697,12 @@ int32 OS_mount (const char *devname, const char* mountpoint) /* mount() cannot be used on this file system at this time */ return_code = OS_ERR_INCORRECT_OBJ_STATE; } - else if ((local->flags & OS_FILESYS_FLAG_IS_FIXED) != 0) + else if (local->system_mountpt[0] == 0) { - /* Won't mount if FIXED, but return SUCCESS for compatibility - * with existing apps/PSP that attempt this operation. */ - return_code = OS_SUCCESS; + /* + * The system mount point should be a non-empty string. + */ + return_code = OS_FS_ERR_PATH_INVALID; } else { @@ -751,12 +778,6 @@ int32 OS_unmount (const char *mountpoint) /* unmount() cannot be used on this file system at this time */ return_code = OS_ERR_INCORRECT_OBJ_STATE; } - else if ((local->flags & OS_FILESYS_FLAG_IS_FIXED) != 0) - { - /* Won't unmount if FIXED, but return SUCCESS for compatibility - * with existing apps/PSP that attempt this operation. */ - return_code = OS_SUCCESS; - } else { return_code = OS_FileSysUnmountVolume_Impl(local_id); diff --git a/src/os/vxworks/src/os-impl-filesys.c b/src/os/vxworks/src/os-impl-filesys.c index c57c1f0ac..4812221db 100644 --- a/src/os/vxworks/src/os-impl-filesys.c +++ b/src/os/vxworks/src/os-impl-filesys.c @@ -81,6 +81,14 @@ int32 OS_FileSysStartVolume_Impl (uint32 filesys_id) return_code = OS_ERR_NOT_IMPLEMENTED; switch(local->fstype) { + case OS_FILESYS_TYPE_FS_BASED: + { + /* pass through for FS_BASED volumes, assume already mounted */ + OS_DEBUG("OSAL: Mapping an FS_BASED disk at: %s\n",(unsigned long)local->system_mountpt ); + return_code = OS_SUCCESS; + break; + } + case OS_FILESYS_TYPE_VOLATILE_DISK: { OS_DEBUG("OSAL: Starting a RAM disk at: 0x%08lX\n",(unsigned long)local->address ); @@ -184,13 +192,24 @@ int32 OS_FileSysStartVolume_Impl (uint32 filesys_id) *-----------------------------------------------------------------*/ int32 OS_FileSysStopVolume_Impl (uint32 filesys_id) { + OS_filesys_internal_record_t *local = &OS_filesys_table[filesys_id]; OS_impl_filesys_internal_record_t *impl = &OS_impl_filesys_table[filesys_id]; - if (impl->xbdMaxPartitions > 0 && impl->xbd != NULLDEV) + switch(local->fstype) { - xbdBlkDevDelete(impl->xbd, NULL); - impl->xbd = NULLDEV; - impl->xbdMaxPartitions = 0; + case OS_FILESYS_TYPE_VOLATILE_DISK: + case OS_FILESYS_TYPE_NORMAL_DISK: + { + if (impl->xbdMaxPartitions > 0 && impl->xbd != NULLDEV) + { + xbdBlkDevDelete(impl->xbd, NULL); + impl->xbd = NULLDEV; + impl->xbdMaxPartitions = 0; + } + break; + } + default: + break; } /* @@ -214,19 +233,43 @@ int32 OS_FileSysStopVolume_Impl (uint32 filesys_id) int32 OS_FileSysFormatVolume_Impl (uint32 filesys_id) { OS_filesys_internal_record_t *local = &OS_filesys_table[filesys_id]; + int32 return_code = OS_ERR_NOT_IMPLEMENTED; int status; - /* - ** Call the dos format routine - */ - status = dosFsVolFormat(local->system_mountpt, DOS_OPT_BLANK, NULL); - if ( status == -1 ) + switch(local->fstype) + { + case OS_FILESYS_TYPE_FS_BASED: + { + /* + * The "format" operation is a no-op on FS_BASED types. + * Return success to allow the operation to continue. + */ + return_code = OS_SUCCESS; + break; + } + case OS_FILESYS_TYPE_VOLATILE_DISK: + case OS_FILESYS_TYPE_NORMAL_DISK: { - OS_DEBUG("OSAL: dosFsVolFormat failed. Errno = %d\n",errnoGet()); - return OS_FS_ERR_DRIVE_NOT_CREATED; + /* + ** Call the dos format routine + */ + status = dosFsVolFormat(local->system_mountpt, DOS_OPT_BLANK, NULL); + if ( status == -1 ) + { + OS_DEBUG("OSAL: dosFsVolFormat failed. Errno = %d\n",errnoGet()); + return_code = OS_FS_ERR_DRIVE_NOT_CREATED; + } + else + { + return_code = OS_SUCCESS; + } + break; + } + default: + break; } - return OS_SUCCESS; + return return_code; } /* end OS_FileSysFormatVolume_Impl */ diff --git a/src/unit-test-coverage/shared/src/coveragetest-filesys.c b/src/unit-test-coverage/shared/src/coveragetest-filesys.c index aa6d64de4..6f60d74ef 100644 --- a/src/unit-test-coverage/shared/src/coveragetest-filesys.c +++ b/src/unit-test-coverage/shared/src/coveragetest-filesys.c @@ -193,18 +193,16 @@ void Test_OS_mount(void) actual = OS_mount("/ramdev5","/ram5"); UtAssert_True(actual == expected, "OS_mount() (%ld) == OS_ERR_NAME_NOT_FOUND", (long)actual); - /* mount on a fixed disk historically returns OS_SUCCESS and is a no-op. - * This is for backward compatibility (it should probably an error to do this) */ - OS_filesys_table[1].flags = OS_FILESYS_FLAG_IS_READY | - OS_FILESYS_FLAG_IS_FIXED; - expected = OS_SUCCESS; + /* Test unknown/unset system mountpoint */ + OS_filesys_table[1].flags = OS_FILESYS_FLAG_IS_READY; + OS_filesys_table[1].system_mountpt[0] = 0; + expected = OS_ERR_NAME_NOT_FOUND; /* should be OS_FS_ERR_PATH_INVALID, but compat return overwrites */ actual = OS_mount("/ramdev5","/ram5"); - UtAssert_True(actual == expected, "OS_mount(fixed) (%ld) == OS_SUCCESS", (long)actual); - + UtAssert_True(actual == expected, "OS_mount(no mountpt) (%ld) == OS_FS_ERR_PATH_INVALID", (long)actual); /* set up so record is in the right state for mounting */ - OS_filesys_table[1].flags = OS_FILESYS_FLAG_IS_READY; expected = OS_SUCCESS; + snprintf(OS_filesys_table[1].system_mountpt, sizeof(OS_filesys_table[1].system_mountpt), "/ut"); actual = OS_mount("/ramdev5","/ram5"); UtAssert_True(actual == expected, "OS_mount(nominal) (%ld) == OS_SUCCESS", (long)actual); @@ -233,16 +231,6 @@ void Test_OS_unmount(void) actual = OS_unmount("/ram0"); UtAssert_True(actual == expected, "OS_mount() (%ld) == OS_ERR_NAME_NOT_FOUND", (long)actual); - /* unmount on a fixed disk historically returns OS_SUCCESS and is a no-op. - * This is for backward compatibility (it should probably an error to do this) */ - OS_filesys_table[1].flags = OS_FILESYS_FLAG_IS_READY | - OS_FILESYS_FLAG_IS_FIXED | - OS_FILESYS_FLAG_IS_MOUNTED_SYSTEM | - OS_FILESYS_FLAG_IS_MOUNTED_VIRTUAL; - expected = OS_SUCCESS; - actual = OS_unmount("/ram0"); - UtAssert_True(actual == expected, "OS_unmount(fixed) (%ld) == OS_SUCCESS", (long)actual); - /* set up so record is in the right state for mounting */ OS_filesys_table[1].flags = OS_FILESYS_FLAG_IS_READY | OS_FILESYS_FLAG_IS_MOUNTED_SYSTEM | diff --git a/src/unit-test-coverage/vxworks/src/coveragetest-filesys.c b/src/unit-test-coverage/vxworks/src/coveragetest-filesys.c index a8233b609..7f029f442 100644 --- a/src/unit-test-coverage/vxworks/src/coveragetest-filesys.c +++ b/src/unit-test-coverage/vxworks/src/coveragetest-filesys.c @@ -43,10 +43,14 @@ void Test_OS_FileSysStartVolume_Impl(void) */ int32 expected; - /* Emulate an FS_BASED entry */ - OS_filesys_table[0].fstype = OS_FILESYS_TYPE_DEFAULT; + /* Emulate an UNKNOWN entry */ + OS_filesys_table[0].fstype = OS_FILESYS_TYPE_UNKNOWN; OSAPI_TEST_FUNCTION_RC(OS_FileSysStartVolume_Impl(0), OS_ERR_NOT_IMPLEMENTED); + /* Emulate an FS_BASED entry */ + OS_filesys_table[0].fstype = OS_FILESYS_TYPE_FS_BASED; + OSAPI_TEST_FUNCTION_RC(OS_FileSysStartVolume_Impl(0), OS_SUCCESS); + /* Emulate a VOLATILE_DISK entry (ramdisk) */ OS_filesys_table[1].fstype = OS_FILESYS_TYPE_VOLATILE_DISK; OSAPI_TEST_FUNCTION_RC(OS_FileSysStartVolume_Impl(1), OS_SUCCESS); @@ -78,6 +82,7 @@ void Test_OS_FileSysStopVolume_Impl(void) OSAPI_TEST_FUNCTION_RC(OS_FileSysStopVolume_Impl(0), OS_SUCCESS); /* Failure to delete XBD layer */ + OS_filesys_table[1].fstype = OS_FILESYS_TYPE_VOLATILE_DISK; UT_FileSysTest_SetupFileSysEntry(1, NULL, 1, 4); OSAPI_TEST_FUNCTION_RC(OS_FileSysStopVolume_Impl(1), OS_SUCCESS); UtAssert_True(UT_GetStubCount(UT_KEY(OCS_xbdBlkDevDelete)) == 1, "xbdBlkDevDelete() called"); @@ -88,6 +93,16 @@ void Test_OS_FileSysFormatVolume_Impl(void) /* Test Case For: * int32 OS_FileSysFormatVolume_Impl (uint32 filesys_id) */ + + /* test unimplemented fs type */ + OS_filesys_table[0].fstype = OS_FILESYS_TYPE_UNKNOWN; + OSAPI_TEST_FUNCTION_RC(OS_FileSysFormatVolume_Impl(0), OS_ERR_NOT_IMPLEMENTED); + + /* fs-based should be noop */ + OS_filesys_table[0].fstype = OS_FILESYS_TYPE_FS_BASED; + OSAPI_TEST_FUNCTION_RC(OS_FileSysFormatVolume_Impl(0), OS_SUCCESS); + + OS_filesys_table[0].fstype = OS_FILESYS_TYPE_VOLATILE_DISK; OSAPI_TEST_FUNCTION_RC(OS_FileSysFormatVolume_Impl(0), OS_SUCCESS); /* Failure of the dosFsVolFormat() call */ From 312b7cc426ab512260d296f687a3b3c1e3553b12 Mon Sep 17 00:00:00 2001 From: Joseph Hickey Date: Wed, 13 May 2020 19:41:27 -0400 Subject: [PATCH 4/4] Fix #453, Select API and unit test updates Confirm that the "selectable" flag is set before calling the underlying select() API. Also update unit tests to match. --- src/os/inc/osapi.h | 1 + src/os/portable/os-impl-bsd-select.c | 23 +++++++++- src/os/shared/inc/os-shared-select.h | 2 + .../portable/src/coveragetest-bsd-select.c | 4 ++ .../oscore-test/ut_oscore_select_test.c | 44 +++++++++++-------- 5 files changed, 53 insertions(+), 21 deletions(-) diff --git a/src/os/inc/osapi.h b/src/os/inc/osapi.h index c34b614be..070950a04 100644 --- a/src/os/inc/osapi.h +++ b/src/os/inc/osapi.h @@ -77,6 +77,7 @@ #define OS_ERR_INCORRECT_OBJ_STATE (-35) /**< @brief Incorrect object state */ #define OS_ERR_INCORRECT_OBJ_TYPE (-36) /**< @brief Incorrect object type */ #define OS_ERR_STREAM_DISCONNECTED (-37) /**< @brief Stream disconnected */ +#define OS_ERR_OPERATION_NOT_SUPPORTED (-38) /**< @brief Requested operation is not support on the supplied object(s) */ /**@}*/ /* diff --git a/src/os/portable/os-impl-bsd-select.c b/src/os/portable/os-impl-bsd-select.c index b7282d811..abfbad33b 100644 --- a/src/os/portable/os-impl-bsd-select.c +++ b/src/os/portable/os-impl-bsd-select.c @@ -82,7 +82,7 @@ static int OS_FdSet_ConvertIn_Impl(fd_set *os_set, OS_FdSet *OSAL_set) { id = (offset * 8) + bit; osfd = OS_impl_filehandle_table[id].fd; - if (osfd >= 0) + if (osfd >= 0 && OS_impl_filehandle_table[id].selectable) { FD_SET(osfd, os_set); if (osfd > maxfd) @@ -247,6 +247,15 @@ int32 OS_SelectSingle_Impl(uint32 stream_id, uint32 *SelectFlags, int32 msecs) fd_set wr_set; fd_set rd_set; + /* + * If called on a stream_id which does not support this + * operation, return immediately and do not invoke the system call + */ + if (!OS_impl_filehandle_table[stream_id].selectable) + { + return OS_ERR_OPERATION_NOT_SUPPORTED; + } + if (*SelectFlags != 0) { FD_ZERO(&wr_set); @@ -304,6 +313,13 @@ int32 OS_SelectMultiple_Impl(OS_FdSet *ReadSet, OS_FdSet *WriteSet, int32 msecs) int maxfd; int32 return_code; + /* + * This return code will be used if the set(s) do not + * contain any file handles capable of select(). It + * will be overwritten with the real result of the + * select call, if selectable file handles were specified. + */ + return_code = OS_ERR_OPERATION_NOT_SUPPORTED; FD_ZERO(&rd_set); FD_ZERO(&wr_set); maxfd = -1; @@ -324,7 +340,10 @@ int32 OS_SelectMultiple_Impl(OS_FdSet *ReadSet, OS_FdSet *WriteSet, int32 msecs) } } - return_code = OS_DoSelect(maxfd, &rd_set, &wr_set, msecs); + if (maxfd >= 0) + { + return_code = OS_DoSelect(maxfd, &rd_set, &wr_set, msecs); + } if (return_code == OS_SUCCESS) { diff --git a/src/os/shared/inc/os-shared-select.h b/src/os/shared/inc/os-shared-select.h index e7f83648f..0f0557429 100644 --- a/src/os/shared/inc/os-shared-select.h +++ b/src/os/shared/inc/os-shared-select.h @@ -42,6 +42,7 @@ Bits in "SelectFlags" will be unset according to activity Returns: OS_SUCCESS on success, or relevant error code + OS_ERR_OPERATION_NOT_SUPPORTED if the specified file handle does not support select ------------------------------------------------------------------*/ int32 OS_SelectSingle_Impl(uint32 stream_id, uint32 *SelectFlags, int32 msecs); @@ -66,6 +67,7 @@ int32 OS_SelectSingle_Impl(uint32 stream_id, uint32 *SelectFlags, int32 msecs); File descriptors in sets be modified according to activity Returns: OS_SUCCESS on success, or relevant error code + OS_ERR_OPERATION_NOT_SUPPORTED if the specified file handle(s) do not support select ------------------------------------------------------------------*/ int32 OS_SelectMultiple_Impl(OS_FdSet *ReadSet, OS_FdSet *WriteSet, int32 msecs); diff --git a/src/unit-test-coverage/portable/src/coveragetest-bsd-select.c b/src/unit-test-coverage/portable/src/coveragetest-bsd-select.c index 10d13d8de..5862eb09a 100644 --- a/src/unit-test-coverage/portable/src/coveragetest-bsd-select.c +++ b/src/unit-test-coverage/portable/src/coveragetest-bsd-select.c @@ -17,6 +17,7 @@ * */ #include "os-portable-coveragetest.h" +#include "ut-adaptor-portable-posix-io.h" #include "os-shared-select.h" #include @@ -32,7 +33,10 @@ void Test_OS_SelectSingle_Impl(void) struct OCS_timespec latertime; StreamID = 0; + UT_PortablePosixIOTest_Set_Selectable(0, false); SelectFlags = OS_STREAM_STATE_READABLE | OS_STREAM_STATE_WRITABLE; + OSAPI_TEST_FUNCTION_RC(OS_SelectSingle_Impl, (StreamID, &SelectFlags, 0), OS_ERR_OPERATION_NOT_SUPPORTED); + UT_PortablePosixIOTest_Set_Selectable(0, true); OSAPI_TEST_FUNCTION_RC(OS_SelectSingle_Impl, (StreamID, &SelectFlags, 0), OS_SUCCESS); SelectFlags = OS_STREAM_STATE_READABLE | OS_STREAM_STATE_WRITABLE; OSAPI_TEST_FUNCTION_RC(OS_SelectSingle_Impl, (StreamID, &SelectFlags, -1), OS_SUCCESS); diff --git a/src/unit-tests/oscore-test/ut_oscore_select_test.c b/src/unit-tests/oscore-test/ut_oscore_select_test.c index c69932147..13caa26ff 100644 --- a/src/unit-tests/oscore-test/ut_oscore_select_test.c +++ b/src/unit-tests/oscore-test/ut_oscore_select_test.c @@ -43,8 +43,8 @@ char *fsAddrPtr = NULL; static int32 setup_file(void) { - OS_mkfs(fsAddrPtr, "/ramdev3", " ", 512, 20); - OS_mount("/ramdev3", "/drive3"); + UT_SETUP(OS_mkfs(fsAddrPtr, "/ramdev3", "RAM3", 512, 20)); + UT_SETUP(OS_mount("/ramdev3", "/drive3")); return OS_creat("/drive3/select_test.txt", OS_READ_WRITE); } @@ -105,20 +105,25 @@ void UT_os_select_single_test(void) { uint32 StateFlags; int32 fd = setup_file(); + int32 rc; - if(OS_SelectSingle(fd, &StateFlags, 0) == OS_ERR_NOT_IMPLEMENTED) + UT_RETVAL(OS_SelectSingle(fd, NULL, 0), OS_INVALID_POINTER, "NULL flags pointer"); + + StateFlags = OS_STREAM_STATE_WRITABLE; + rc = OS_SelectSingle(fd, &StateFlags, 0); + if(rc == OS_ERR_NOT_IMPLEMENTED || rc == OS_ERR_OPERATION_NOT_SUPPORTED) { - UtAssertEx(false, UTASSERT_CASETYPE_NA, __FILE__, __LINE__, "OS_SelectSingle() not implemented"); + UtAssertEx(false, UTASSERT_CASETYPE_NA, __FILE__, __LINE__, "OS_SelectSingle() not supported"); goto UT_os_select_single_test_exit_tag; } - UtAssert_Simple(OS_SelectSingle(fd, NULL, 0) != OS_SUCCESS); - - StateFlags = OS_STREAM_STATE_WRITABLE; - UtAssert_Simple(OS_SelectSingle(fd, &StateFlags, 0) == OS_SUCCESS && StateFlags & OS_STREAM_STATE_WRITABLE); + UT_RETVAL(rc, OS_SUCCESS, "OS_SelectSingle(fd, OS_STREAM_STATE_WRITABLE, 0)"); + UtAssert_True((StateFlags & OS_STREAM_STATE_WRITABLE) != 0, "StateFlags (0x%x) & OS_STREAM_STATE_WRITABLE", (unsigned int)StateFlags); StateFlags = OS_STREAM_STATE_READABLE; - UtAssert_Simple(OS_SelectSingle(fd, &StateFlags, 1) == OS_SUCCESS); + rc = OS_SelectSingle(fd, &StateFlags, 1); + UT_RETVAL(rc, OS_SUCCESS, "OS_SelectSingle(fd, OS_STREAM_STATE_READABLE, 0)"); + UtAssert_True((StateFlags & OS_STREAM_STATE_READABLE) != 0, "StateFlags (0x%x) & OS_STREAM_STATE_READABLE", (unsigned int)StateFlags); UT_os_select_single_test_exit_tag: teardown_file(fd); @@ -135,25 +140,26 @@ void UT_os_select_multi_test(void) { OS_FdSet ReadSet, WriteSet; int32 fd = setup_file(); + int32 rc; - if(OS_SelectMultiple(&ReadSet, &WriteSet, 1) == OS_ERR_NOT_IMPLEMENTED) + OS_SelectFdZero(&WriteSet); + OS_SelectFdAdd(&WriteSet, fd); + rc = OS_SelectMultiple(NULL, &WriteSet, 1); + if(rc == OS_ERR_NOT_IMPLEMENTED || rc == OS_ERR_OPERATION_NOT_SUPPORTED) { - UtAssertEx(false, UTASSERT_CASETYPE_NA, __FILE__, __LINE__, "OS_SelectMultiple() not implemented"); + UtAssertEx(false, UTASSERT_CASETYPE_NA, __FILE__, __LINE__, "OS_SelectMultiple() not supported"); goto UT_select_multi_test_exit_tag; } - OS_SelectFdZero(&WriteSet); - OS_SelectFdAdd(&WriteSet, fd); - UtAssert_Simple(OS_SelectMultiple(NULL, &WriteSet, 1) == OS_SUCCESS); - OS_SelectFdZero(&ReadSet); - OS_SelectFdAdd(&ReadSet, fd); - UtAssert_Simple(OS_SelectMultiple(&ReadSet, NULL, 1) == OS_SUCCESS); + UT_RETVAL(rc, OS_SUCCESS, "OS_SelectMultiple(NULL, &WriteSet, 1)"); + UtAssert_True(OS_SelectFdIsSet(&WriteSet, fd), "OS_SelectFdIsSet(&WriteSet, fd)"); OS_SelectFdZero(&ReadSet); OS_SelectFdAdd(&ReadSet, fd); - OS_SelectFdZero(&WriteSet); - UtAssert_Simple(OS_SelectMultiple(&ReadSet, &WriteSet, 0) == OS_SUCCESS); + rc = OS_SelectMultiple(&ReadSet, NULL, 1); + UT_RETVAL(rc, OS_SUCCESS, "OS_SelectMultiple(&ReadSet, NULL, 1)"); + UtAssert_True(OS_SelectFdIsSet(&ReadSet, fd), "!OS_SelectFdIsSet(&ReadSet, fd)"); UT_select_multi_test_exit_tag: teardown_file(fd);