From c00e14faa0b3cbc0768fc47b7c6de46c3851a6dd Mon Sep 17 00:00:00 2001 From: Leor Bleier Date: Fri, 8 May 2020 09:22:57 -0400 Subject: [PATCH 01/20] Fix #447 - more doxygen warnings --- src/os/inc/osapi-os-filesys.h | 23 +++++++++++------------ src/os/portable/os-impl-posix-gettime.c | 5 ++--- 2 files changed, 13 insertions(+), 15 deletions(-) diff --git a/src/os/inc/osapi-os-filesys.h b/src/os/inc/osapi-os-filesys.h index 5739a66f1..1a13d9a32 100644 --- a/src/os/inc/osapi-os-filesys.h +++ b/src/os/inc/osapi-os-filesys.h @@ -1,12 +1,12 @@ /* ** File: osapi-os-filesys.h ** -** Copyright (c) 2004-2006, United States government as represented by the -** administrator of the National Aeronautics Space Administration. -** All rights reserved. This software was created at NASAs Goddard +** Copyright (c) 2004-2006, United States government as represented by the +** administrator of the National Aeronautics Space Administration. +** All rights reserved. This software was created at NASAs Goddard ** Space Flight Center pursuant to government contracts. ** -** This is governed by the NASA Open Source Agreement and may be used, +** This is governed by the NASA Open Source Agreement and may be used, ** distributed and modified only pursuant to the terms of that agreement. ** ** Author: Alan Cudmore Code 582 @@ -53,7 +53,7 @@ #define NUM_TABLE_ENTRIES 14 /* -** Length of a Device and Volume name +** Length of a Device and Volume name */ #define OS_FS_DEV_NAME_LEN 32 /**< Device name length */ #define OS_FS_PHYS_NAME_LEN 64 /**< Physical drive name length */ @@ -86,8 +86,8 @@ #define OS_FS_ERR_PATH_INVALID (-108) /**< @brief FS path invalid */ #ifndef OSAL_OMIT_DEPRECATED -/* - * Map some codes used by the file API back to the generic counterparts +/* + * Map some codes used by the file API back to the generic counterparts * where there is overlap between them. Do not duplicate error codes. */ #define OS_FS_SUCCESS OS_SUCCESS /**< @deprecated Successful execution */ @@ -105,7 +105,7 @@ * * Implementation note for developers: * - * os_fs_err_name_t is now equivalent to the OSAL "os_err_name_t" typedef, + * os_fs_err_name_t is now equivalent to the OSAL "os_err_name_t" typedef, * to preserve source code compatibility with anything using the OS_FS_GetErrorName api * * The sizes of strings in OSAL functions are built with os_fs_err_name_t's @@ -116,7 +116,7 @@ typedef os_err_name_t os_fs_err_name_t; #endif /** - * @brief Internal structure of the OS volume table for + * @brief Internal structure of the OS volume table for * mounted file systems and path translation */ typedef struct @@ -263,9 +263,9 @@ int32 OS_creat (const char *path, int32 access); * * @param[in] path File name to create * @param[in] access Intended access mode - see @ref OSFileAccess - * @param[in] mode The file permissions. This parameter is passed through to the + * @param[in] mode The file permissions. This parameter is passed through to the * native open call, but will be ignored. The file mode (or permissions) - * are ignored by the POSIX open call when the O_CREAT access flag is not passed in. + * are ignored by the POSIX open call when the O_CREAT access flag is not passed in. * * @note Valid handle IDs are never negative. Failure of this * call can be checked by testing if the result is less than 0. @@ -781,7 +781,6 @@ int32 OS_FileSysAddFixedMap(uint32 *filesys_id, const char *phys_path, * @param[in] numblocks The number of blocks to allocate for the drive * * @return Execution status, see @ref OSReturnCodes - * @retval #OS_SUCCESS @copybrief OS_SUCCESS * @retval #OS_INVALID_POINTER if devname is NULL * @retval #OS_FS_ERR_DRIVE_NOT_CREATED if the OS calls to create the the drive failed * @retval #OS_FS_ERR_DEVICE_NOT_FREE if the volume table is full diff --git a/src/os/portable/os-impl-posix-gettime.c b/src/os/portable/os-impl-posix-gettime.c index 95ceb7ee1..259ff382d 100644 --- a/src/os/portable/os-impl-posix-gettime.c +++ b/src/os/portable/os-impl-posix-gettime.c @@ -3,7 +3,7 @@ * administrator of the National Aeronautics Space Administration. * All rights reserved. This software was created at NASA Glenn * Research Center pursuant to government contracts. - * + * * This is governed by the NASA Open Source Agreement and may be used, * distributed and modified only according to the terms of that agreement. */ @@ -15,7 +15,7 @@ * This file contains implementation for OS_GetTime() and OS_SetTime() * that map to the C library clock_gettime() and clock_settime() calls. * This should be usable on any OS that supports those standard calls. - * The OS-specific code must #include the correct headers that define the + * The OS-specific code must \#include the correct headers that define the * prototypes for these functions before including this implementation file. * */ @@ -112,4 +112,3 @@ int32 OS_SetLocalTime_Impl(const OS_time_t *time_struct) return ReturnCode; } /* end OS_SetLocalTime_Impl */ - From 8352d2fa7c2965d40acbe57a5333037f1d57a360 Mon Sep 17 00:00:00 2001 From: Yasir Khan Date: Wed, 6 May 2020 13:36:01 -0400 Subject: [PATCH 02/20] Fix #374, Add idmap functional tests --- src/tests/idmap-api-test/idmap-api-test.c | 278 ++++++++++++++++++++++ 1 file changed, 278 insertions(+) create mode 100644 src/tests/idmap-api-test/idmap-api-test.c diff --git a/src/tests/idmap-api-test/idmap-api-test.c b/src/tests/idmap-api-test/idmap-api-test.c new file mode 100644 index 000000000..2da08c309 --- /dev/null +++ b/src/tests/idmap-api-test/idmap-api-test.c @@ -0,0 +1,278 @@ +#include +#include +#include + +#include "common_types.h" +#include "osapi.h" +#include "utassert.h" +#include "uttest.h" +#include "utbsp.h" + + +/* *************************************** MAIN ************************************** */ + +typedef struct +{ + uint32 TaskCount; + uint32 QueueCount; + uint32 CountSemCount; + uint32 BinSemCount; + uint32 MutexCount; + uint32 TimeBaseCount; + uint32 OtherCount; +} Test_OS_ObjTypeCount_t; + +static void ObjTypeCounter(uint32 object_id, void *arg) +{ + Test_OS_ObjTypeCount_t *count = arg; + + switch(OS_IdentifyObject(object_id)) + { + case OS_OBJECT_TYPE_OS_TASK: + ++count->TaskCount; + break; + case OS_OBJECT_TYPE_OS_QUEUE: + ++count->QueueCount; + break; + case OS_OBJECT_TYPE_OS_COUNTSEM: + ++count->CountSemCount; + break; + case OS_OBJECT_TYPE_OS_BINSEM: + ++count->BinSemCount; + break; + case OS_OBJECT_TYPE_OS_MUTEX: + ++count->MutexCount; + break; + case OS_OBJECT_TYPE_OS_TIMEBASE: + ++count->TimeBaseCount; + break; + default: + ++count->OtherCount; + break; + } +} + +/* +* A void test function that creates an object for testing +*/ +void Test_Void_Fn(void) +{ + uint32 bin_sem_id_my_task; + OS_BinSemCreate( &bin_sem_id_my_task, "BinSemTaskMyTask", 1, 0); + OS_TaskDelay(5); + +} /* end Test_Void_Fn */ + + +/* *************************************** MAIN ************************************** */ + +void TestIdMapApi(void) +{ + int32 expected; + int32 actual; + uint32 task_id; + uint32 queue_id; + uint32 count_sem_id; + uint32 bin_sem_id; + uint32 mutex_id1; + uint32 mutex_id2; + uint32 mutex_id3; + uint32 time_base_id; + uint32 TestArrayIndex; + Test_OS_ObjTypeCount_t Count; + + /* + * Create all allowed objects + */ + OS_TaskCreate( &task_id, "Task", Test_Void_Fn, 0, 0, 0, 0); + OS_QueueCreate( &queue_id, "Queue", 5, 5, 0); + OS_CountSemCreate( &count_sem_id, "CountSem", 1, 0); + OS_BinSemCreate( &bin_sem_id, "BinSem", 1, 0); + OS_MutSemCreate( &mutex_id1, "Mutex1", 0); + OS_MutSemCreate( &mutex_id2, "Mutex2", 0); + OS_MutSemCreate( &mutex_id3, "Mutex3", 0); + OS_TimeBaseCreate( &time_base_id, "TimeBase", 0); + + /* + * NOTE: The following objects were not created and tested: + * OS_OBJECT_TYPE_OS_STREAM + * OS_OBJECT_TYPE_OS_DIR + * OS_OBJECT_TYPE_OS_TIMECB + * OS_OBJECT_TYPE_OS_FILESYS + * OS_OBJECT_TYPE_OS_CONSOLE + * OS_OBJECT_TYPE_USER + */ + + /* + * Test Case For: + * int32 OS_IdentifyObject(void) + */ + + /* + * Test with nominal values + */ + expected = OS_OBJECT_TYPE_OS_TASK; + actual = OS_IdentifyObject(task_id); + UtAssert_True(actual == expected, "OS_IdentifyObject() (%ld) == %ld", (long)actual, (long)expected); + + expected = OS_OBJECT_TYPE_OS_QUEUE; + actual = OS_IdentifyObject(queue_id); + UtAssert_True(actual == expected, "OS_IdentifyObject() (%ld) == %ld", (long)actual, (long)expected); + + expected = OS_OBJECT_TYPE_OS_COUNTSEM; + actual = OS_IdentifyObject(count_sem_id); + UtAssert_True(actual == expected, "OS_IdentifyObject() (%ld) == %ld", (long)actual, (long)expected); + + expected = OS_OBJECT_TYPE_OS_BINSEM; + actual = OS_IdentifyObject(bin_sem_id); + UtAssert_True(actual == expected, "OS_IdentifyObject() (%ld) == %ld", (long)actual, (long)expected); + + expected = OS_OBJECT_TYPE_OS_MUTEX; + actual = OS_IdentifyObject(mutex_id1); + UtAssert_True(actual == expected, "OS_IdentifyObject() (%ld) == %ld", (long)actual, (long)expected); + + expected = OS_OBJECT_TYPE_OS_TIMEBASE; + actual = OS_IdentifyObject(time_base_id); + UtAssert_True(actual == expected, "OS_IdentifyObject() (%ld) == %ld", (long)actual, (long)expected); + + /* + * Test with extreme cases using min and max values + * Note: There are no asserts, checks or expected values + * here. The only check is that the function doesn't return + * an error when called + */ + OS_IdentifyObject(0x00000); + OS_IdentifyObject(0xFFFFFFFF); + + /* + * Test Case For: + * int32 OS_ConvertToArrayIndex(void) + */ + expected = OS_SUCCESS; + + /* + * Check different id types and verify array indices + * Each Object Type index is added to an array index of its own type + * Each object type is checked once, and MUTEX is checked twice to + * verify multiple indices + */ + + /* + * Test with nominal values + */ + actual = OS_ConvertToArrayIndex(task_id, &TestArrayIndex); + UtAssert_True(actual == expected , "OS_ConvertToArrayIndex() (%ld) == %ld ", (long)actual, (long)expected ); + UtAssert_True(TestArrayIndex == 1 , "TestArrayIndex(%lu) == 1", (long)TestArrayIndex); + + actual = OS_ConvertToArrayIndex(queue_id, &TestArrayIndex); + UtAssert_True(actual == expected , "OS_ConvertToArrayIndex() (%ld) == %ld ", (long)actual, (long)expected ); + UtAssert_True(TestArrayIndex == 1 , "TestArrayIndex(%lu) == 1", (long)TestArrayIndex); + + actual = OS_ConvertToArrayIndex(count_sem_id, &TestArrayIndex); + UtAssert_True(actual == expected , "OS_ConvertToArrayIndex() (%ld) == %ld ", (long)actual, (long)expected ); + UtAssert_True(TestArrayIndex == 1 , "TestArrayIndex(%lu) == 1", (long)TestArrayIndex); + + actual = OS_ConvertToArrayIndex(bin_sem_id, &TestArrayIndex); + UtAssert_True(actual == expected , "OS_ConvertToArrayIndex() (%ld) == %ld ", (long)actual, (long)expected ); + UtAssert_True(TestArrayIndex == 2 , "TestArrayIndex(%lu) == 2", (long)TestArrayIndex); + + actual = OS_ConvertToArrayIndex(mutex_id1, &TestArrayIndex); + UtAssert_True(actual == expected , "OS_ConvertToArrayIndex() (%ld) == %ld ", (long)actual, (long)expected ); + UtAssert_True(TestArrayIndex == 1 , "TestArrayIndex(%lu) == 1", (long)TestArrayIndex); + + actual = OS_ConvertToArrayIndex(mutex_id2, &TestArrayIndex); + UtAssert_True(actual == expected , "OS_ConvertToArrayIndex() (%ld) == %ld ", (long)actual, (long)expected ); + UtAssert_True(TestArrayIndex == 2 , "TestArrayIndex(%lu) == 2", (long)TestArrayIndex); + + actual = OS_ConvertToArrayIndex(mutex_id3, &TestArrayIndex); + UtAssert_True(actual == expected , "OS_ConvertToArrayIndex() (%ld) == %ld ", (long)actual, (long)expected ); + UtAssert_True(TestArrayIndex == 3 , "TestArrayIndex(%lu) == 3", (long)TestArrayIndex); + + actual = OS_ConvertToArrayIndex(time_base_id, &TestArrayIndex); + UtAssert_True(actual == expected , "OS_ConvertToArrayIndex() (%ld) == %ld ", (long)actual, (long)expected ); + UtAssert_True(TestArrayIndex == 1 , "TestArrayIndex(%lu) == 1", (long)TestArrayIndex); + + /* + * Test with extreme cases using invalid inputs and checking + * for an error return code + */ + actual = OS_ConvertToArrayIndex(0x0000, &TestArrayIndex); + expected = OS_ERR_INCORRECT_OBJ_TYPE; + UtAssert_True(actual == expected , "OS_ConvertToArrayIndex() (%ld) == %ld ", (long)actual, (long)expected ); + + actual = OS_ConvertToArrayIndex(0xFFFFFFFF, &TestArrayIndex); + expected = OS_ERR_INCORRECT_OBJ_TYPE; + UtAssert_True(actual == expected , "OS_ConvertToArrayIndex() (%ld) == %ld ", (long)actual, (long)expected ); + + /* + * Test Case For: + * void OS_ForEachObject (uint32 creator_id, OS_ArgCallback_t callback_ptr, void *callback_arg); + */ + memset(&Count, 0, sizeof(Count)); + + OS_ForEachObject (0, &ObjTypeCounter, &Count); + + /* Verify Outputs */ + UtAssert_True(Count.TaskCount == 1, "OS_ForEachObject() TaskCount (%lu) == 1", (unsigned long)Count.TaskCount); + UtAssert_True(Count.QueueCount == 1, "OS_ForEachObject() QueueCount (%lu) == 1", (unsigned long)Count.QueueCount); + UtAssert_True(Count.CountSemCount == 1, "OS_ForEachObject() CountSemCount (%lu) == 1", (unsigned long)Count.CountSemCount); + UtAssert_True(Count.BinSemCount == 2, "OS_ForEachObject() BinSemCount (%lu) == 2", (unsigned long)Count.BinSemCount); + UtAssert_True(Count.MutexCount == 3, "OS_ForEachObject() MutexCount (%lu) == 3", (unsigned long)Count.MutexCount); + UtAssert_True(Count.TimeBaseCount == 1, "OS_ForEachObject() TimeBaseCount (%lu) == 1", (unsigned long)Count.TimeBaseCount); + + /* + * Use current task as an input + */ + memset(&Count, 0, sizeof(Count)); + OS_ForEachObject (task_id, &ObjTypeCounter, &Count); + + /* Verify Output */ + UtAssert_True(Count.BinSemCount == 1, "OS_ForEachObject() BinSemCount MyTask (%lu) == 1", (unsigned long)Count.BinSemCount); + + /* + * Delete all created objects, and verify that the count is now zero + */ + memset(&Count, 0, sizeof(Count)); + OS_DeleteAllObjects(); + OS_ForEachObject (0, &ObjTypeCounter, &Count); + + /* Verify Outputs */ + UtAssert_True(Count.TaskCount == 0, "OS_ForEachObject() TaskCount After Delete (%lu) == 0", (unsigned long)Count.TaskCount); + UtAssert_True(Count.QueueCount == 0, "OS_ForEachObject() QueueCount After Delete (%lu) == 0", (unsigned long)Count.QueueCount); + UtAssert_True(Count.CountSemCount == 0, "OS_ForEachObject() CountSemCount After Delete (%lu) == 0", (unsigned long)Count.CountSemCount); + UtAssert_True(Count.BinSemCount == 0, "OS_ForEachObject() BinSemCount After Delete (%lu) == 0", (unsigned long)Count.BinSemCount); + UtAssert_True(Count.MutexCount == 0, "OS_ForEachObject() MutexCount After Delete (%lu) == 0", (unsigned long)Count.MutexCount); + UtAssert_True(Count.TimeBaseCount == 0, "OS_ForEachObject() TimeBaseCount After Delete (%lu) == 0", (unsigned long)Count.TimeBaseCount); + + /* + * Pass an invalid input, and verify that object counts are not increased + */ + OS_ForEachObject (0xFFFFFFFF, &ObjTypeCounter, &Count); + + /* Verify Outputs */ + UtAssert_True(Count.TaskCount == 0, "OS_ForEachObject() TaskCount Invalid Input (%lu) == 0", (unsigned long)Count.TaskCount); + UtAssert_True(Count.QueueCount == 0, "OS_ForEachObject() QueueCount Invalid Input (%lu) == 0", (unsigned long)Count.QueueCount); + UtAssert_True(Count.CountSemCount == 0, "OS_ForEachObject() CountSemCount Invalid Input (%lu) == 0", (unsigned long)Count.CountSemCount); + UtAssert_True(Count.BinSemCount == 0, "OS_ForEachObject() BinSemCount Invalid Input (%lu) == 0", (unsigned long)Count.BinSemCount); + UtAssert_True(Count.MutexCount == 0, "OS_ForEachObject() MutexCount Invalid Input (%lu) == 0", (unsigned long)Count.MutexCount); + UtAssert_True(Count.TimeBaseCount == 0, "OS_ForEachObject() TimeBaseCount Invalid Input (%lu) == 0", (unsigned long)Count.TimeBaseCount); + + + + +} /* end TestIdMapApi */ + + +void UtTest_Setup(void) +{ + if (OS_API_Init() != OS_SUCCESS) + { + UtAssert_Abort("OS_API_Init() failed"); + } + + /* + * Register the test setup and check routines in UT assert + */ + UtTest_Add(TestIdMapApi, NULL, NULL, "TestIdMapApi"); +} + From 45c9742c2a81a04666208fac78c37498b0b38d6a Mon Sep 17 00:00:00 2001 From: Joseph Hickey Date: Wed, 13 May 2020 20:28:11 -0400 Subject: [PATCH 03/20] Fix #454, Better error translation on statvfs() The statvfs() call depends on the underlying filesystem supporting this. On RTEMS, the IMFS filesystem type does not, and it returns the ENOSYS errno. This is better translated to OS_ERR_NOT_IMPLEMENTED rather than OS_ERROR. --- src/os/rtems/src/os-impl-filesys.c | 27 ++++++++++++++++++++++----- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/src/os/rtems/src/os-impl-filesys.c b/src/os/rtems/src/os-impl-filesys.c index 40d127522..fb4534c2a 100644 --- a/src/os/rtems/src/os-impl-filesys.c +++ b/src/os/rtems/src/os-impl-filesys.c @@ -376,17 +376,34 @@ int32 OS_FileSysStatVolume_Impl (uint32 filesys_id, OS_statvfs_t *result) { OS_filesys_internal_record_t *local = &OS_filesys_table[filesys_id]; struct statvfs stat_buf; + int32 return_code; if ( statvfs(local->system_mountpt, &stat_buf) != 0 ) { - return OS_ERROR; + /* + * The ENOSYS error means it is not implemented at the system level. + * This should translate to the OS_ERR_NOT_IMPLEMENTED OSAL code. + */ + if (errno == ENOSYS) + { + return_code = OS_ERR_NOT_IMPLEMENTED; + } + else + { + OS_DEBUG("%s: %s\n", local->system_mountpt, strerror(errno)); + return_code = OS_ERROR; + } } + else + { + result->block_size = stat_buf.f_bsize; + result->blocks_free = stat_buf.f_bfree; + result->total_blocks = stat_buf.f_blocks; - result->block_size = stat_buf.f_bsize; - result->blocks_free = stat_buf.f_bfree; - result->total_blocks = stat_buf.f_blocks; + return_code = OS_SUCCESS; + } - return(OS_SUCCESS); + return (return_code); } /* end OS_FileSysStatVolume_Impl */ From 5f97b9b74e2dbe916bdf77b45b1039ab65f38f52 Mon Sep 17 00:00:00 2001 From: Joseph Hickey Date: Wed, 13 May 2020 21:09:14 -0400 Subject: [PATCH 04/20] Fix #459, dynamically create RAM disk devices on RTEMS Rather than relying on the BSP to preallocate, the ram disk block devices can be created based on request. This correlates with the way RAM disks are implemented on VxWorks and is cleaner and more flexible by making it more independent of the BSP. --- src/bsp/pc-rtems/src/bsp_start.c | 23 ------ src/bsp/pc-rtems/src/pcrtems_bsp_internal.h | 1 - src/os/rtems/src/os-impl-filesys.c | 87 +++++++++------------ src/unit-tests/osfile-test/ut_osfile_test.c | 2 +- 4 files changed, 40 insertions(+), 73 deletions(-) diff --git a/src/bsp/pc-rtems/src/bsp_start.c b/src/bsp/pc-rtems/src/bsp_start.c index bcce3f6e2..cece37736 100644 --- a/src/bsp/pc-rtems/src/bsp_start.c +++ b/src/bsp/pc-rtems/src/bsp_start.c @@ -47,29 +47,6 @@ extern rtems_status_code rtems_ide_part_table_initialize(const char *); extern int rtems_rtl_shell_command (int argc, char* argv[]); -/* - * The RAM Disk configuration. - */ -rtems_ramdisk_config rtems_ramdisk_configuration[RTEMS_NUMBER_OF_RAMDISKS]; - -/* - * The number of RAM Disk configurations. - */ -size_t rtems_ramdisk_configuration_size = RTEMS_NUMBER_OF_RAMDISKS; - -/* -** RAM Disk IO op table. -*/ -rtems_driver_address_table rtems_ramdisk_io_ops = -{ - .initialization_entry = ramdisk_initialize, - .open_entry = rtems_blkdev_generic_open, - .close_entry = rtems_blkdev_generic_close, - .read_entry = rtems_blkdev_generic_read, - .write_entry = rtems_blkdev_generic_write, - .control_entry = rtems_blkdev_generic_ioctl -}; - /* * Additional shell commands for the RTL functionality */ diff --git a/src/bsp/pc-rtems/src/pcrtems_bsp_internal.h b/src/bsp/pc-rtems/src/pcrtems_bsp_internal.h index 89656d2c7..cf969525d 100644 --- a/src/bsp/pc-rtems/src/pcrtems_bsp_internal.h +++ b/src/bsp/pc-rtems/src/pcrtems_bsp_internal.h @@ -28,7 +28,6 @@ * BSP compile-time tuning */ #define RTEMS_MAX_USER_OPTIONS 4 -#define RTEMS_NUMBER_OF_RAMDISKS 1 #define RTEMS_MAX_CMDLINE 256 /* diff --git a/src/os/rtems/src/os-impl-filesys.c b/src/os/rtems/src/os-impl-filesys.c index 40d127522..4003cf484 100644 --- a/src/os/rtems/src/os-impl-filesys.c +++ b/src/os/rtems/src/os-impl-filesys.c @@ -46,7 +46,8 @@ typedef struct { char blockdev_name[OS_MAX_PATH_LEN]; - rtems_device_minor_number minor; + + struct ramdisk *allocated_disk; /* other data to pass to "mount" when mounting this disk */ const char *mount_fstype; @@ -65,19 +66,6 @@ typedef struct */ OS_impl_filesys_internal_record_t OS_impl_filesys_table[OS_MAX_FILE_SYSTEMS]; - -/* - * These external references are for the RTEMS RAM disk device descriptor table - * This is necessary for the RAM disk. These tables can either be here, or - * in a RTEMS kernel startup file. In this case, the tables are in the - * application startup - * - * Currently, it does not appear possible to create multiple arbitrary disks - * The RAM disk driver appears to require these specific variables. - */ -extern rtems_ramdisk_config rtems_ramdisk_configuration[]; -extern size_t rtems_ramdisk_configuration_size; - /**************************************************************************************** Filesys API ***************************************************************************************/ @@ -110,7 +98,7 @@ int32 OS_FileSysStartVolume_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]; - uint32 os_idx; + rtems_status_code sc; int32 return_code; return_code = OS_ERR_NOT_IMPLEMENTED; @@ -136,48 +124,41 @@ int32 OS_FileSysStartVolume_Impl (uint32 filesys_id) } case OS_FILESYS_TYPE_VOLATILE_DISK: { - /* - * This finds the correct driver "minor number" - * to use for the RAM disk (i.e. /dev/rd) - */ + OS_DEBUG("No RAMDISK available at address %p\n", local->address); - /* find a matching entry in the OS ramdisk table, - * (identified by the location address) */ - for (os_idx = 0; os_idx < rtems_ramdisk_configuration_size; ++os_idx) - { - if (rtems_ramdisk_configuration[os_idx].location == local->address) - { - impl->minor = os_idx; - break; - } - } + impl->allocated_disk = ramdisk_allocate( + local->address, + local->blocksize, + local->numblocks, + false + ); - if (os_idx >= rtems_ramdisk_configuration_size) + if (impl->allocated_disk == NULL) { - OS_DEBUG("No RAMDISK available at address %p\n", local->address); + OS_DEBUG("ramdisk_allocate() failed\n"); return_code = OS_INVALID_POINTER; break; } - if ( local->numblocks > rtems_ramdisk_configuration[os_idx].block_num) - { - OS_DEBUG("OSAL: Error: RAM disk too large, %lu blocks requested, %lu available.\n", - (unsigned long)local->numblocks, - (unsigned long)rtems_ramdisk_configuration[os_idx].block_num); - return_code = OS_ERROR; - break; - } - if ( local->blocksize != rtems_ramdisk_configuration[os_idx].block_size ) + + impl->mount_fstype = RTEMS_FILESYSTEM_TYPE_RFS; + impl->mount_options = RTEMS_FILESYSTEM_READ_WRITE; + snprintf(impl->blockdev_name, sizeof(impl->blockdev_name), "%s%c", RAMDISK_DEVICE_BASE_NAME, (int)filesys_id + 'a'); + + sc = rtems_blkdev_create( + impl->blockdev_name, + local->blocksize, + local->numblocks, + ramdisk_ioctl, + impl->allocated_disk + ); + if (sc != RTEMS_SUCCESSFUL) { - OS_DEBUG("OSAL: Error: RAM Disk needs a block size of %lu.\n", - (unsigned long)rtems_ramdisk_configuration[os_idx].block_size); - return_code = OS_ERROR; - break; + OS_DEBUG("rtems_blkdev_create() failed: %s.\n", rtems_status_text(sc)); + return_code = OS_ERROR; } - snprintf(impl->blockdev_name, sizeof(impl->blockdev_name), "%s%c", RAMDISK_DEVICE_BASE_NAME, (int)impl->minor + 'a'); - impl->mount_fstype = RTEMS_FILESYSTEM_TYPE_RFS; - OS_DEBUG("OSAL: RAM disk initialized: volume=%s device=%s address=0x%08lX\n", + OS_DEBUG("RAM disk initialized: volume=%s device=%s address=0x%08lX\n", local->volume_name, impl->blockdev_name, (unsigned long)local->address); return_code = OS_SUCCESS; @@ -218,7 +199,16 @@ int32 OS_FileSysStartVolume_Impl (uint32 filesys_id) *-----------------------------------------------------------------*/ int32 OS_FileSysStopVolume_Impl (uint32 filesys_id) { - /* Currently nothing to do here */ + OS_impl_filesys_internal_record_t *impl = &OS_impl_filesys_table[filesys_id]; + + /* + * If this was a dynamically allocated disk, then unlink it. + */ + if (impl->allocated_disk != NULL) + { + unlink(impl->blockdev_name); + } + return OS_SUCCESS; } /* end OS_FileSysStopVolume_Impl */ @@ -265,6 +255,7 @@ int32 OS_FileSysFormatVolume_Impl (uint32 filesys_id) ** Format the RAM disk with the RFS file system */ memset (&config, 0, sizeof(config)); + config.inode_overhead = 30; sc = rtems_rfs_format(impl->blockdev_name, &config); if ( sc < 0 ) { diff --git a/src/unit-tests/osfile-test/ut_osfile_test.c b/src/unit-tests/osfile-test/ut_osfile_test.c index 8a511a73b..4cc5ae3f9 100644 --- a/src/unit-tests/osfile-test/ut_osfile_test.c +++ b/src/unit-tests/osfile-test/ut_osfile_test.c @@ -56,7 +56,7 @@ int32 UT_os_setup_fs() { int32 res; - res = OS_mkfs(g_fsAddrPtr, g_devName, " ", 512, 20); + res = OS_mkfs(g_fsAddrPtr, g_devName, "RAM3", 512, 64); if (res != OS_SUCCESS) { UT_OS_LOG("OS_mkfs() returns %d\n", (int)res);; From 72af6d57dd7bf3522e2868c419a925554e5bd8ab Mon Sep 17 00:00:00 2001 From: Joseph Hickey Date: Wed, 13 May 2020 21:23:58 -0400 Subject: [PATCH 05/20] Fix #455, correct length of device_name in filesys This string should be of OS_FS_DEV_NAME_LEN --- src/os/shared/inc/os-shared-filesys.h | 2 +- src/os/shared/src/osapi-filesys.c | 4 ++-- src/unit-test-coverage/shared/src/coveragetest-filesys.c | 8 +++++--- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/os/shared/inc/os-shared-filesys.h b/src/os/shared/inc/os-shared-filesys.h index d578e57bf..dab908db8 100644 --- a/src/os/shared/inc/os-shared-filesys.h +++ b/src/os/shared/inc/os-shared-filesys.h @@ -95,7 +95,7 @@ typedef struct typedef struct { - char device_name[OS_MAX_API_NAME]; /**< The name of the underlying block device, if applicable */ + char device_name[OS_FS_DEV_NAME_LEN]; /**< The name of the underlying block device, if applicable */ char volume_name[OS_FS_VOL_NAME_LEN]; char system_mountpt[OS_MAX_LOCAL_PATH_LEN]; /**< The name/prefix where the contents are accessible in the host operating system */ char virtual_mountpt[OS_MAX_PATH_LEN]; /**< The name/prefix in the OSAL Virtual File system exposed to applications */ diff --git a/src/os/shared/src/osapi-filesys.c b/src/os/shared/src/osapi-filesys.c index c2638ad70..cf6724bc9 100644 --- a/src/os/shared/src/osapi-filesys.c +++ b/src/os/shared/src/osapi-filesys.c @@ -443,7 +443,7 @@ int32 OS_FileSysAddFixedMap(uint32 *filesys_id, const char *phys_path, const cha return OS_INVALID_POINTER; } - if (strlen(phys_path) >= OS_MAX_PATH_LEN || + if (strlen(phys_path) >= OS_MAX_LOCAL_PATH_LEN || strlen(virt_path) >= OS_MAX_PATH_LEN) { return OS_ERR_NAME_TOO_LONG; @@ -462,7 +462,7 @@ int32 OS_FileSysAddFixedMap(uint32 *filesys_id, const char *phys_path, const cha ++dev_name; } - if (strlen(dev_name) >= OS_MAX_API_NAME) + if (strlen(dev_name) >= OS_FS_DEV_NAME_LEN) { return OS_ERR_NAME_TOO_LONG; } diff --git a/src/unit-test-coverage/shared/src/coveragetest-filesys.c b/src/unit-test-coverage/shared/src/coveragetest-filesys.c index e84d14ec7..fcf40a969 100644 --- a/src/unit-test-coverage/shared/src/coveragetest-filesys.c +++ b/src/unit-test-coverage/shared/src/coveragetest-filesys.c @@ -51,12 +51,14 @@ void Test_OS_FileSysAddFixedMap(void) OSAPI_TEST_FUNCTION_RC(OS_FileSysAddFixedMap(&id, "/phys", "/virt"), OS_SUCCESS); OSAPI_TEST_FUNCTION_RC(OS_FileSysAddFixedMap(&id, NULL, NULL), OS_INVALID_POINTER); - UT_SetForceFail(UT_KEY(OCS_strlen), 2 + OS_MAX_PATH_LEN); + UT_SetDeferredRetcode(UT_KEY(OCS_strlen), 1, 2 + OS_MAX_LOCAL_PATH_LEN); OSAPI_TEST_FUNCTION_RC(OS_FileSysAddFixedMap(&id, "/phys", "/virt"), OS_ERR_NAME_TOO_LONG); - UT_ClearForceFail(UT_KEY(OCS_strlen)); + UT_SetDeferredRetcode(UT_KEY(OCS_strlen), 2, 2 + OS_MAX_PATH_LEN); + OSAPI_TEST_FUNCTION_RC(OS_FileSysAddFixedMap(&id, "/phys", "/virt"), OS_ERR_NAME_TOO_LONG); + UT_ResetState(UT_KEY(OCS_strlen)); UT_SetForceFail(UT_KEY(OCS_strrchr), -1); - UT_SetDeferredRetcode(UT_KEY(OCS_strlen), 3, 2 + OS_MAX_API_NAME); + UT_SetDeferredRetcode(UT_KEY(OCS_strlen), 3, 2 + OS_FS_DEV_NAME_LEN); OSAPI_TEST_FUNCTION_RC(OS_FileSysAddFixedMap(&id, "/phys", "/virt"), OS_ERR_NAME_TOO_LONG); UT_ResetState(UT_KEY(OCS_strlen)); UT_ResetState(UT_KEY(OCS_strrchr)); From 042db6b157b9c72cdccab0bfd3fe3e029104668d Mon Sep 17 00:00:00 2001 From: Joseph Hickey Date: Thu, 14 May 2020 08:06:51 -0400 Subject: [PATCH 06/20] Fix #457, provide BSP shutdown handler This provides a better method of handling test abort, in a BSP-specific manner. --- src/bsp/mcp750-vxworks/src/bsp_start.c | 11 +++++++ src/bsp/pc-linux/src/bsp_start.c | 11 +++++++ src/bsp/pc-rtems/src/bsp_start.c | 41 ++++++++++++++++++-------- src/bsp/shared/inc/bsp-impl.h | 10 +++++++ ut_assert/src/utbsp.c | 4 +-- 5 files changed, 62 insertions(+), 15 deletions(-) diff --git a/src/bsp/mcp750-vxworks/src/bsp_start.c b/src/bsp/mcp750-vxworks/src/bsp_start.c index 6c47c4eaf..4669e7a8b 100644 --- a/src/bsp/mcp750-vxworks/src/bsp_start.c +++ b/src/bsp/mcp750-vxworks/src/bsp_start.c @@ -24,6 +24,17 @@ #include "mcp750_bsp_internal.h" +/* --------------------------------------------------------- + OS_BSP_Shutdown_Impl() + + Helper function to abort the running task + --------------------------------------------------------- */ +void OS_BSP_Shutdown_Impl(void) +{ + abort(); +} + + /****************************************************************************** ** Function: OS_BSPMain() ** diff --git a/src/bsp/pc-linux/src/bsp_start.c b/src/bsp/pc-linux/src/bsp_start.c index 50d080fcb..e886eda41 100644 --- a/src/bsp/pc-linux/src/bsp_start.c +++ b/src/bsp/pc-linux/src/bsp_start.c @@ -132,6 +132,17 @@ int OS_BSP_GetReturnStatus(void) return retcode; } +/* --------------------------------------------------------- + OS_BSP_Shutdown_Impl() + + Helper function to abort the running task + --------------------------------------------------------- */ +void OS_BSP_Shutdown_Impl(void) +{ + abort(); +} + + /****************************************************************************** ** Function: main() ** diff --git a/src/bsp/pc-rtems/src/bsp_start.c b/src/bsp/pc-rtems/src/bsp_start.c index bcce3f6e2..78ffefacc 100644 --- a/src/bsp/pc-rtems/src/bsp_start.c +++ b/src/bsp/pc-rtems/src/bsp_start.c @@ -324,6 +324,30 @@ rtems_status_code OS_BSP_GetReturnStatus(void) return retcode; } +/* --------------------------------------------------------- + OS_BSP_Shutdown_Impl() + + Helper function to abort the running task + --------------------------------------------------------- */ +void OS_BSP_Shutdown_Impl(void) +{ + /* + * Not calling exit() under RTEMS, this simply shuts down the executive, + * forcing the user to reboot the system. + * + * Calling suspend causes execution to get stuck here, but the RTEMS + * shell thread will still be active so the user can poke around, read results, + * then use a shell command to reboot when ready. + */ + while (!OS_BSP_PcRtemsGlobal.BatchMode) + { + printf("\n\nInit thread idle.\nPress for shell or reset machine...\n\n"); + rtems_task_suspend(rtems_task_self()); + } + + rtems_shutdown_executive(OS_BSP_GetReturnStatus()); +} + /* ** A simple entry point to start from the loader */ @@ -355,20 +379,11 @@ rtems_task Init(rtems_task_argument ignored) OS_Application_Run(); /* - * Not calling exit() under RTEMS, this simply shuts down the executive, - * forcing the user to reboot the system. - * - * Calling suspend causes execution to get stuck here, but the RTEMS - * shell thread will still be active so the user can poke around, read results, - * then use a shell command to reboot when ready. + * Enter the BSP default shutdown mode + * depending on config, this may reset/reboot or suspend + * so the operator can use the shell. */ - while (!OS_BSP_PcRtemsGlobal.BatchMode) - { - printf("\n\nInit thread idle.\nPress for shell or reset machine...\n\n"); - rtems_task_suspend(rtems_task_self()); - } - - rtems_shutdown_executive(OS_BSP_GetReturnStatus()); + OS_BSP_Shutdown_Impl(); } /* configuration information */ diff --git a/src/bsp/shared/inc/bsp-impl.h b/src/bsp/shared/inc/bsp-impl.h index bce51a9bb..39b0ad775 100644 --- a/src/bsp/shared/inc/bsp-impl.h +++ b/src/bsp/shared/inc/bsp-impl.h @@ -133,6 +133,16 @@ void OS_BSP_ConsoleOutput_Impl(const char *Str, uint32 DataLen); ------------------------------------------------------------------*/ void OS_BSP_ConsoleSetMode_Impl(uint32 ModeBits); +/*---------------------------------------------------------------- + Function: OS_BSP_Shutdown_Impl + + Purpose: Causes the calling task to abort in a BSP-safe way. + This may map to the abort() system call, but on some systems + that causes a reboot or undesirable side effect. The + BSP may implement this call in a different manner. + ------------------------------------------------------------------*/ +void OS_BSP_Shutdown_Impl(void); + /********************* END bsp-impl.h *********************/ diff --git a/ut_assert/src/utbsp.c b/ut_assert/src/utbsp.c index 1744af8c2..f63e57923 100644 --- a/ut_assert/src/utbsp.c +++ b/ut_assert/src/utbsp.c @@ -170,11 +170,11 @@ void UT_BSP_DoText(uint8 MessageType, const char *OutputMessage) /* * If any ABORT (major failure) message is thrown, - * then actually call abort() to stop the test and dump a core + * then call a BSP-provided routine to stop the test and possibly dump a core */ if (MessageType == UTASSERT_CASETYPE_ABORT) { - abort(); + OS_BSP_Shutdown_Impl(); } } From 1c36569232264983974320b9c6335477929707e5 Mon Sep 17 00:00:00 2001 From: Joseph Hickey Date: Thu, 14 May 2020 14:41:22 -0400 Subject: [PATCH 07/20] 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 08/20] 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 09/20] 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 10/20] 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); From c139ae38f64911575466d7c1280425da7696c575 Mon Sep 17 00:00:00 2001 From: Joseph Hickey Date: Fri, 15 May 2020 23:11:16 -0400 Subject: [PATCH 11/20] Fix #450, add external source directory for OS/BSP If the "OSAL_EXT_SOURCE_DIR" cache variable is set, this location will be checked first for a BSP/OS implementation layer. This can point to an out-of-tree implementation layer if necessary. However it is discouraged from actually doing this as there is no attempt at API stability at the low level implementation layer. --- CMakeLists.txt | 52 ++++++++++++++++++++++++++++++++++---------------- 1 file changed, 36 insertions(+), 16 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index e378f56ce..77fee09f7 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -48,6 +48,15 @@ cmake_minimum_required(VERSION 2.8.12) project(OSAL C) +# The "OSAL_EXT_SOURCE_DIR" cache variable may be set to a path +# on the host containing extra OS/BSP implementations which are not +# part of the open source release. +# CAUTION: The API between the OSAL and the low level implementation and/or BSP +# is not stabilized, and may change with every OSAL release. No attempt is made +# to provide backward compatibility with to external sources. +set(OSAL_EXT_SOURCE_DIR "$ENV{OSAL_EXT_SOURCE_DIR}" + CACHE PATH "External source directory to check for additional OS/BSP implementations") + # Read the default compile-time configuration, and update with # any mission/project specific options in the OSAL_CONFIGURATION_FILE include("${OSAL_SOURCE_DIR}/default_config.cmake") @@ -104,15 +113,20 @@ add_subdirectory(ut_assert) # OSAL_SYSTEM_BSPTYPE indicate which of the BSP packages # to build. These is required and must be defined. Confirm that this exists # and error out now if it does not. -if (NOT DEFINED OSAL_SYSTEM_BSPTYPE OR - NOT IS_DIRECTORY "${OSAL_SOURCE_DIR}/src/bsp/${OSAL_SYSTEM_BSPTYPE}") - # It is an error if the indicated BSPTYPE does not correspond to a subdirectory - # If this is not caught here then a more obfuscated error will occur later. - message("Error: \"${OSAL_SYSTEM_BSPTYPE}\" is not a valid BSP type") +if (NOT DEFINED OSAL_SYSTEM_BSPTYPE) message(FATAL_ERROR "OSAL_SYSTEM_BSPTYPE must be set to the appropriate BSP") endif () +if (OSAL_EXT_SOURCE_DIR AND IS_DIRECTORY "${OSAL_EXT_SOURCE_DIR}/${OSAL_SYSTEM_BSPTYPE}") + set(OSAL_BSP_SOURCE_DIR "${OSAL_EXT_SOURCE_DIR}/${OSAL_SYSTEM_BSPTYPE}") +elseif(IS_DIRECTORY "${OSAL_SOURCE_DIR}/src/bsp/${OSAL_SYSTEM_BSPTYPE}") + set(OSAL_BSP_SOURCE_DIR "${OSAL_SOURCE_DIR}/src/bsp/${OSAL_SYSTEM_BSPTYPE}") +else() + # It is an error if the indicated BSPTYPE does not correspond to a subdirectory + # If this is not caught here then a more obfuscated error will occur later. + message(FATAL_ERROR "Error: No source directory found for \"${OSAL_SYSTEM_BSPTYPE}\" BSP type") +endif() -message(STATUS "BSP Selection: ${OSAL_SYSTEM_BSPTYPE}") +message(STATUS "BSP Selection: ${OSAL_SYSTEM_BSPTYPE} at ${OSAL_BSP_SOURCE_DIR}") # The BSP library is a separate target from OSAL and can be used @@ -121,7 +135,7 @@ message(STATUS "BSP Selection: ${OSAL_SYSTEM_BSPTYPE}") # # The Implementation-Specific BSP subdirectory should define # an OBJECT target named "osal_${OSAL_SYSTEM_BSPTYPE}_impl" -add_subdirectory(src/bsp/${OSAL_SYSTEM_BSPTYPE} ${OSAL_SYSTEM_BSPTYPE}_impl) +add_subdirectory(${OSAL_BSP_SOURCE_DIR} ${OSAL_SYSTEM_BSPTYPE}_impl) target_include_directories(osal_${OSAL_SYSTEM_BSPTYPE}_impl PRIVATE ${OSAL_SOURCE_DIR}/src/bsp/shared/inc ) @@ -187,19 +201,25 @@ target_include_directories(osal_bsp PRIVATE # OSAL_SYSTEM_OSTYPE indicates which of the OS packages # to build. If not defined, this may be inferred by the BSP type. -if (NOT DEFINED OSAL_SYSTEM_OSTYPE OR - NOT IS_DIRECTORY "${OSAL_SOURCE_DIR}/src/os/${OSAL_SYSTEM_OSTYPE}") - # It is an error if the indicated OSTYPE does not correspond to a subdirectory - # If this is not caught here then a more obfuscated error will occur later. - message("Error: \"${OSAL_SYSTEM_OSTYPE}\" is not a valid OS type") +if (NOT DEFINED OSAL_SYSTEM_OSTYPE) message(FATAL_ERROR "OSAL_SYSTEM_OSTYPE must be set to the appropriate OS") endif () +if (OSAL_EXT_SOURCE_DIR AND IS_DIRECTORY "${OSAL_EXT_SOURCE_DIR}/${OSAL_SYSTEM_OSTYPE}") + set(OSAL_OS_SOURCE_DIR "${OSAL_EXT_SOURCE_DIR}/${OSAL_SYSTEM_OSTYPE}") +elseif(IS_DIRECTORY "${OSAL_SOURCE_DIR}/src/os/${OSAL_SYSTEM_OSTYPE}") + set(OSAL_OS_SOURCE_DIR "${OSAL_SOURCE_DIR}/src/os/${OSAL_SYSTEM_OSTYPE}") +else() + # It is an error if the indicated OSTYPE does not correspond to a subdirectory + # If this is not caught here then a more obfuscated error will occur later. + message(FATAL_ERROR "Error: No source directory found for \"${OSAL_SYSTEM_OSTYPE}\" OS type") +endif() + -message(STATUS "OSAL Selection: ${OSAL_SYSTEM_OSTYPE}") +message(STATUS "OSAL Selection: ${OSAL_SYSTEM_OSTYPE} at ${OSAL_OS_SOURCE_DIR}") # The implementation-specific OSAL subdirectory should define # an OBJECT target named "osal_${OSAL_SYSTEM_OSTYPE}_impl" -add_subdirectory(src/os/${OSAL_SYSTEM_OSTYPE} ${OSAL_SYSTEM_OSTYPE}_impl) +add_subdirectory(${OSAL_OS_SOURCE_DIR} ${OSAL_SYSTEM_OSTYPE}_impl) # The "shared" directory contains internal components which # are referenced in implementation OSAL modules, but should _NOT_ @@ -290,8 +310,8 @@ endif(OSAL_BSP_INCLUDE_DIRECTORIES) # fine-tune the library for this particular build. This is included # AFTER The basic targets are defined, so it may set properties # on the defined targets and/or use target-specific commands. -include("${OSAL_SOURCE_DIR}/src/bsp/${OSAL_SYSTEM_BSPTYPE}/build_options.cmake" OPTIONAL) -include("${OSAL_SOURCE_DIR}/src/os/${OSAL_SYSTEM_OSTYPE}/build_options.cmake" OPTIONAL) +include("${OSAL_BSP_SOURCE_DIR}/build_options.cmake" OPTIONAL) +include("${OSAL_OS_SOURCE_DIR}/build_options.cmake" OPTIONAL) # # UNIT TEST SUPPORT From 36ae15382ce7e079e96d1393944e8feb2c4cc3fb Mon Sep 17 00:00:00 2001 From: Joseph Hickey Date: Mon, 30 Mar 2020 17:25:07 -0400 Subject: [PATCH 12/20] Fix #293, Expand API for object queries Implement OS_GetResourceName, OS_ForEachObjectByType --- src/os/inc/osapi-os-core.h | 48 +++- src/os/shared/inc/os-shared-idmap.h | 32 +++ src/os/shared/src/osapi-idmap.c | 220 +++++++++++------- src/os/shared/src/osapi-time.c | 2 +- src/tests/osal-core-test/osal-core-test.c | 94 ++++++++ .../shared/src/coveragetest-idmap.c | 100 ++++---- src/ut-stubs/osapi-utstub-idmap.c | 57 +++++ 7 files changed, 415 insertions(+), 138 deletions(-) diff --git a/src/os/inc/osapi-os-core.h b/src/os/inc/osapi-os-core.h index a9c1f336d..dab1fcf03 100644 --- a/src/os/inc/osapi-os-core.h +++ b/src/os/inc/osapi-os-core.h @@ -47,6 +47,13 @@ /** @brief Upper limit for OSAL task priorities */ #define OS_MAX_TASK_PRIORITY 255 +/** + * @brief Constant that may be passed to OS_ForEachObject()/OS_ForEachObjectOfType() to match any + * creator (i.e. get all objects) + */ +#define OS_OBJECT_CREATOR_ANY 0 + + /** @defgroup OSSemaphoreStates OSAL Semaphore State Defines * @{ */ @@ -278,6 +285,24 @@ void OS_ApplicationExit(int32 Status); * @{ */ +/*-------------------------------------------------------------------------------------*/ +/** + * @brief Obtain the name of an object given an arbitrary object ID + * + * All OSAL resources generally have a name associated with them. This + * allows application code to retrieve the name of any valid OSAL object ID. + * + * @param[in] object_id The object ID to operate on + * @param[out] buffer Buffer in which to store the name + * @param[in] buffer_size Size of the output storage buffer + * + * @returns #OS_SUCCESS if successful + * #OS_ERR_INVALID_ID if the passed-in ID is not a valid OSAL ID + * #OS_INVALID_POINTER if the passed-in buffer is invalid + * #OS_ERR_NAME_TOO_LONG if the name will not fit in the buffer provided + */ +int32 OS_GetResourceName(uint32 id, char *buffer, uint32 buffer_size); + /*-------------------------------------------------------------------------------------*/ /** * @brief Obtain the type of an object given an arbitrary object ID @@ -315,12 +340,33 @@ int32 OS_ConvertToArrayIndex (uint32 object_id, uint32 *ArrayIndex); /** * @brief call the supplied callback function for all valid object IDs * - * Loops through all defined OSAL objects and calls callback_ptr on each one + * Loops through all defined OSAL objects of all types and calls callback_ptr on each one * If creator_id is nonzero then only objects with matching creator id are processed. + * + * @param[in] creator_id Filter objects to those created by a specific task + * This may be passed as OS_OBJECT_CREATOR_ANY to return all objects + * @param[in] callback_ptr Function to invoke for each matching object ID + * @param[in] callback_arg Opaque Argument to pass to callback function */ void OS_ForEachObject (uint32 creator_id, OS_ArgCallback_t callback_ptr, void *callback_arg); /**@}*/ +/*-------------------------------------------------------------------------------------*/ +/** + * @brief call the supplied callback function for valid object IDs of a specific type + * + * Loops through all defined OSAL objects of a specific type and calls callback_ptr on each one + * If creator_id is nonzero then only objects with matching creator id are processed. + * + * @param[in] objtype The type of objects to iterate + * @param[in] creator_id Filter objects to those created by a specific task + * This may be passed as OS_OBJECT_CREATOR_ANY to return all objects + * @param[in] callback_ptr Function to invoke for each matching object ID + * @param[in] callback_arg Opaque Argument to pass to callback function + */ +void OS_ForEachObjectOfType (uint32 objtype, uint32 creator_id, OS_ArgCallback_t callback_ptr, void *callback_arg); + + /** @defgroup OSAPITask OSAL Task APIs * @{ */ diff --git a/src/os/shared/inc/os-shared-idmap.h b/src/os/shared/inc/os-shared-idmap.h index 402cf0a46..085c718a0 100644 --- a/src/os/shared/inc/os-shared-idmap.h +++ b/src/os/shared/inc/os-shared-idmap.h @@ -129,6 +129,38 @@ int32 OS_Unlock_Global_Impl(uint32 idtype); corresponding index within the local tables. */ +/*---------------------------------------------------------------- + Function: OS_ObjectIdToSerialNumber + + Purpose: Obtain the serial number component of a generic OSAL Object ID + ------------------------------------------------------------------*/ +static inline uint32 OS_ObjectIdToSerialNumber_Impl(uint32 id) +{ + return (id & OS_OBJECT_INDEX_MASK); +} + +/*---------------------------------------------------------------- + Function: OS_ObjectIdToType + + Purpose: Obtain the object type component of a generic OSAL Object ID + ------------------------------------------------------------------*/ +static inline uint32 OS_ObjectIdToType_Impl(uint32 id) +{ + return (id >> OS_OBJECT_TYPE_SHIFT); +} + + +/*---------------------------------------------------------------- + Function: OS_ObjectIdCompose + + Purpose: Convert an object serial number and resource type into an external 32-bit OSAL ID + ------------------------------------------------------------------*/ +static inline void OS_ObjectIdCompose_Impl(uint32 idtype, uint32 idserial, uint32 *result) +{ + *result = (idtype << OS_OBJECT_TYPE_SHIFT) | idserial; +} + + /*---------------------------------------------------------------- Function: OS_GetMaxForObjectType diff --git a/src/os/shared/src/osapi-idmap.c b/src/os/shared/src/osapi-idmap.c index 7f2ea0097..53398d477 100644 --- a/src/os/shared/src/osapi-idmap.c +++ b/src/os/shared/src/osapi-idmap.c @@ -112,47 +112,6 @@ int32 OS_ObjectIdInit(void) return OS_SUCCESS; } /* end OS_ObjectIdInit */ -/*---------------------------------------------------------------- - * - * Function: OS_ObjectIdMap - * - * Purpose: Local helper routine, not part of OSAL API. - * - *-----------------------------------------------------------------*/ -int32 OS_ObjectIdMap(uint32 idtype, uint32 idvalue, uint32 *result) -{ - *result = (idtype << OS_OBJECT_TYPE_SHIFT) | idvalue; - - if (idtype == OS_OBJECT_TYPE_UNDEFINED || - (idvalue & ~OS_OBJECT_INDEX_MASK) != 0) - { - return OS_ERR_INVALID_ID; - } - - return OS_SUCCESS; -} /* end OS_ObjectIdMap */ - - -/*---------------------------------------------------------------- - * - * Function: OS_ObjectIdUnMap - * - * Purpose: Local helper routine, not part of OSAL API. - * - *-----------------------------------------------------------------*/ -int32 OS_ObjectIdUnMap(uint32 id, uint32 idtype, uint32 *idvalue) -{ - *idvalue = id & OS_OBJECT_INDEX_MASK; - - if ((id >> OS_OBJECT_TYPE_SHIFT) != idtype) - { - return OS_ERR_INVALID_ID; - } - - return OS_SUCCESS; -} /* end OS_ObjectIdUnMap */ - - /*---------------------------------------------------------------- * * Function: OS_GetMaxForObjectType @@ -530,7 +489,7 @@ int32 OS_ObjectIdFindNext(uint32 idtype, uint32 *array_index, OS_common_record_t if(return_code == OS_SUCCESS) { - return_code = OS_ObjectIdMap(idtype, idvalue, &obj->active_id); + OS_ObjectIdCompose_Impl(idtype, idvalue, &obj->active_id); /* Ensure any data in the record has been cleared */ obj->name_entry = NULL; @@ -575,6 +534,9 @@ int32 OS_ObjectIdFindNext(uint32 idtype, uint32 *array_index, OS_common_record_t * for use as an array index. The array index will be in the range of: * 0 <= ArrayIndex < OS_MAX_ * + * If the passed-in ID type is OS_OBJECT_TYPE_UNDEFINED, then any type + * is allowed. + * * returns: If the passed-in ID is not of the proper type, OS_ERROR is returned * Otherwise OS_SUCCESS is returned. * @@ -582,17 +544,33 @@ int32 OS_ObjectIdFindNext(uint32 idtype, uint32 *array_index, OS_common_record_t int32 OS_ObjectIdToArrayIndex(uint32 idtype, uint32 id, uint32 *ArrayIndex) { uint32 max_id; + uint32 obj_index; + uint32 actual_type; int32 return_code; - max_id = OS_GetMaxForObjectType(idtype); - if (max_id == 0) + obj_index = OS_ObjectIdToSerialNumber_Impl(id); + actual_type = OS_ObjectIdToType_Impl(id); + + /* + * If requested by the caller, enforce that the ID is of the correct type. + * If the caller passed OS_OBJECT_TYPE_UNDEFINED, then anything is allowed. + */ + if (idtype != OS_OBJECT_TYPE_UNDEFINED && actual_type != idtype) { - return_code = OS_ERR_INVALID_ID; + return_code = OS_ERR_INVALID_ID; } else { - return_code = OS_ObjectIdUnMap(id, idtype, &id); - *ArrayIndex = id % max_id; + max_id = OS_GetMaxForObjectType(actual_type); + if (max_id == 0) + { + return_code = OS_ERR_INVALID_ID; + } + else + { + return_code = OS_SUCCESS; + *ArrayIndex = obj_index % max_id; + } } return return_code; @@ -962,21 +940,8 @@ int32 OS_ObjectIdAllocateNew(uint32 idtype, const char *name, uint32 *array_inde *-----------------------------------------------------------------*/ int32 OS_ConvertToArrayIndex(uint32 object_id, uint32 *ArrayIndex) { - uint32 max_id; - int32 return_code; - - max_id = OS_GetMaxForObjectType(object_id >> OS_OBJECT_TYPE_SHIFT); - if (max_id == 0) - { - return_code = OS_ERR_INCORRECT_OBJ_TYPE; - } - else - { - *ArrayIndex = (object_id & OS_OBJECT_INDEX_MASK) % max_id; - return_code = OS_SUCCESS; - } - - return return_code; + /* just pass to the generic internal conversion routine */ + return OS_ObjectIdToArrayIndex(OS_OBJECT_TYPE_UNDEFINED, object_id, ArrayIndex); } /* end OS_ConvertToArrayIndex */ @@ -989,40 +954,73 @@ int32 OS_ConvertToArrayIndex(uint32 object_id, uint32 *ArrayIndex) * *-----------------------------------------------------------------*/ void OS_ForEachObject (uint32 creator_id, OS_ArgCallback_t callback_ptr, void *callback_arg) +{ + uint32 idtype; + + for (idtype = 0; idtype < OS_OBJECT_TYPE_USER; ++idtype) + { + OS_ForEachObjectOfType(idtype, creator_id, callback_ptr, callback_arg); + } +} /* end OS_ForEachObject */ + +/*----------------------------------------------------------------- + * + * Function: OS_ForEachObjectOfType + * + * Purpose: Implemented per public OSAL API + * See description in API and header file for detail + * + *-----------------------------------------------------------------*/ +void OS_ForEachObjectOfType (uint32 idtype, uint32 creator_id, OS_ArgCallback_t callback_ptr, void *callback_arg) { uint32 obj_index; uint32 obj_max; uint32 obj_id; - uint32 idtype; - for (idtype = 0; idtype < OS_OBJECT_TYPE_USER; ++idtype) + obj_max = OS_GetMaxForObjectType(idtype); + if (obj_max > 0) { - obj_max = OS_GetMaxForObjectType(idtype); - if (obj_max > 0) + obj_index = OS_GetBaseForObjectType(idtype); + OS_Lock_Global_Impl(idtype); + while (obj_max > 0) { - OS_Lock_Global_Impl(idtype); - obj_index = OS_GetBaseForObjectType(idtype); - while (obj_max > 0) + /* + * Check if the obj_id is both valid and matches + * the specified creator_id + */ + obj_id = OS_common_table[obj_index].active_id; + if (obj_id != 0 && creator_id != OS_OBJECT_CREATOR_ANY && + OS_common_table[obj_index].creator != creator_id) { - obj_id = OS_common_table[obj_index].active_id; - if (obj_id != 0 && (creator_id == 0 || OS_common_table[obj_index].creator == creator_id)) - { - /* - * Handle the object - Note that we must UN-lock before callback. - * The callback function might lock again in a different manner. - */ - OS_Unlock_Global_Impl(idtype); - (*callback_ptr)(obj_id, callback_arg); - OS_Lock_Global_Impl(idtype); + /* valid object but not a creator match - + * skip the callback for this object */ + obj_id = 0; + } - } - ++obj_index; - --obj_max; + if (obj_id != 0) + { + /* + * Invoke Callback for the object, which must be done + * while the global table is unlocked. + * + * Note this means by the time the callback is done, + * the object could have been deleted by another task. + * + * But this must not invoke a callback with a locked table, + * as the callback function might call other OSAL functions, + * which could deadlock. + */ + OS_Unlock_Global_Impl(idtype); + (*callback_ptr)(obj_id, callback_arg); + OS_Lock_Global_Impl(idtype); } - OS_Unlock_Global_Impl(idtype); + + ++obj_index; + --obj_max; } + OS_Unlock_Global_Impl(idtype); } -} /* end OS_ForEachObject */ +} /* end OS_ForEachObjectOfType */ /*---------------------------------------------------------------- * @@ -1034,6 +1032,58 @@ void OS_ForEachObject (uint32 creator_id, OS_ArgCallback_t callback_ptr, void *c *-----------------------------------------------------------------*/ uint32 OS_IdentifyObject (uint32 object_id) { - return (object_id >> OS_OBJECT_TYPE_SHIFT); + return OS_ObjectIdToType_Impl(object_id); } /* end OS_IdentifyObject */ +/*---------------------------------------------------------------- + * + * Function: OS_GetResourceName + * + * Purpose: Implemented per public OSAL API + * See description in API and header file for detail + * + *-----------------------------------------------------------------*/ +int32 OS_GetResourceName(uint32 id, char *buffer, uint32 buffer_size) +{ + uint32 idtype; + OS_common_record_t *record; + int32 return_code; + uint32 name_len; + uint32 local_id; + + /* sanity check the passed-in buffer and size */ + if (buffer == NULL || buffer_size == 0) + { + return OS_INVALID_POINTER; + } + + /* + * Initially set the output string to empty. + * This avoids undefined behavior in case the function fails + * and the caller does not check the return code. + */ + buffer[0] = 0; + + idtype = OS_ObjectIdToType_Impl(id); + return_code = OS_ObjectIdGetById(OS_LOCK_MODE_GLOBAL, idtype, id, &local_id, &record); + if (return_code == OS_SUCCESS) + { + if (record->name_entry != NULL) + { + name_len = strlen(record->name_entry); + if (buffer_size <= name_len) + { + /* indicates the name does not fit into supplied buffer */ + return_code = OS_ERR_NAME_TOO_LONG; + name_len = buffer_size - 1; + } + memcpy(buffer, record->name_entry, name_len); + buffer[name_len] = 0; + } + OS_Unlock_Global_Impl(idtype); + } + + return return_code; +} /* end OS_GetResourceName */ + + diff --git a/src/os/shared/src/osapi-time.c b/src/os/shared/src/osapi-time.c index 4089e5a4c..67598bf26 100644 --- a/src/os/shared/src/osapi-time.c +++ b/src/os/shared/src/osapi-time.c @@ -436,7 +436,7 @@ int32 OS_TimerDelete(uint32 timer_id) { if (local->next_ref != local_id) { - OS_ObjectIdMap(OS_OBJECT_TYPE_OS_TIMEBASE, local->next_ref, &OS_timebase_table[local->timebase_ref].first_cb); + OS_ObjectIdCompose_Impl(OS_OBJECT_TYPE_OS_TIMEBASE, local->next_ref, &OS_timebase_table[local->timebase_ref].first_cb); } else { diff --git a/src/tests/osal-core-test/osal-core-test.c b/src/tests/osal-core-test/osal-core-test.c index a1d0d8ac8..df079abb0 100644 --- a/src/tests/osal-core-test/osal-core-test.c +++ b/src/tests/osal-core-test/osal-core-test.c @@ -13,6 +13,14 @@ #define UT_EXIT_LOOP_MAX 100 /* Used to limit wait for self-exiting task to exit */ /* OS Constructs */ +#define OSAL_UT_MAX_CALLBACKS 5 +typedef struct +{ + uint32 NumInvocations; + uint32 ObjList[OSAL_UT_MAX_CALLBACKS]; +} TestCallbackState_t; + + void TestTasks (void); void InitializeTaskIds (void); @@ -23,6 +31,20 @@ void TestQueues(void); void TestBinaries(void); void TestMutexes(void); void TestGetInfos(void); +void TestGenericQueries(void); + +/* helper function for "OS_ForEachObject" test cases */ +static void TestForEachCallback(uint32 object_id, void *arg) +{ + TestCallbackState_t *State = (TestCallbackState_t*)arg; + if (State->NumInvocations < OSAL_UT_MAX_CALLBACKS) + { + State->ObjList[State->NumInvocations] = object_id; + } + ++State->NumInvocations; +} + + /* *************************************** MAIN ************************************** */ void UtTest_Setup(void) @@ -37,6 +59,7 @@ void UtTest_Setup(void) UtTest_Add(TestBinaries, NULL, NULL, "BSEM"); UtTest_Add(TestMutexes, NULL, NULL, "MSEM"); UtTest_Add(TestGetInfos, NULL, NULL, "INFO"); + UtTest_Add(TestGenericQueries, NULL, NULL, "QUERIES"); } /* end OS_Application Startup */ @@ -592,3 +615,74 @@ void TestGetInfos(void) UtAssert_True(status == OS_SUCCESS, "OS_MutSemDelete"); } +/* + * Validate generic query functions such as OS_ForEachObject, OS_GetResourceName, + * and any other function that operates on any generic/unspecified object type. + */ +void TestGenericQueries(void) +{ + int status; + TestCallbackState_t State; + char ResourceName[OS_MAX_API_NAME]; + + status = OS_TaskCreate( &task_0_id, "Task 0", task_0, task_0_stack, + TASK_0_STACK_SIZE, TASK_0_PRIORITY, 0); + UtAssert_True(status == OS_SUCCESS, "OS_TaskCreate (%ld) == OS_SUCCESS", (long)status); + + status = OS_QueueCreate( &msgq_0, "q 0", MSGQ_DEPTH, MSGQ_SIZE, 0); + UtAssert_True(status == OS_SUCCESS, "OS_QueueCreate (%ld) == OS_SUCCESS", (long)status); + + status = OS_BinSemCreate( &bin_0,"Bin 0",1,0); + UtAssert_True(status == OS_SUCCESS, "OS_BinSemCreate (%ld) == OS_SUCCESS", (long)status); + + status = OS_MutSemCreate( &mut_0,"Mut 0",0); + UtAssert_True(status == OS_SUCCESS, "OS_MutSemCreate (%ld) == OS_SUCCESS", (long)status); + + /* The "OS_ForEachObjectOfType()" should callback for only a specific object type - + * spot check for Tasks and Bin Sem */ + memset(&State, 0, sizeof(State)); + OS_ForEachObjectOfType(OS_OBJECT_TYPE_OS_TASK, OS_OBJECT_CREATOR_ANY, TestForEachCallback, &State); + UtAssert_True(State.NumInvocations == 1, "Task NumInvocations (%lu) == 1", (unsigned long)State.NumInvocations); + UtAssert_True(State.ObjList[0] == task_0_id, "Task ObjList[0] (%lx) == %lx", + (unsigned long)State.ObjList[0], (unsigned long)task_0_id); + + memset(&State, 0, sizeof(State)); + OS_ForEachObjectOfType(OS_OBJECT_TYPE_OS_BINSEM, OS_OBJECT_CREATOR_ANY, TestForEachCallback, &State); + UtAssert_True(State.NumInvocations == 1, "BinSem NumInvocations (%lu) == 1", (unsigned long)State.NumInvocations); + UtAssert_True(State.ObjList[0] == bin_0, "BinSem ObjList[0] (%lx) == %lx", + (unsigned long)State.ObjList[0], (unsigned long)bin_0); + + memset(&State, 0, sizeof(State)); + OS_ForEachObjectOfType(OS_OBJECT_TYPE_OS_COUNTSEM, OS_OBJECT_CREATOR_ANY, TestForEachCallback, &State); + UtAssert_True(State.NumInvocations == 0, "CountSem NumInvocations (%lu) == 0", (unsigned long)State.NumInvocations); + + /* + * The generic "OS_ForEachObject()" should callback for all object types. + * + * Note there are internally-generated object IDs that also may get returned here, + * in addition to the ones which are explicitly created within this test. Therefore + * it is only verified that the callback was invoked for _at least_ the resources + * created here, but OK if it is more than that. + */ + memset(&State, 0, sizeof(State)); + OS_ForEachObject(OS_OBJECT_CREATOR_ANY, TestForEachCallback, &State); + UtAssert_True(State.NumInvocations >= 4, "State.NumInvocations (%lu) >= 4", (unsigned long)State.NumInvocations); + + /* Test the OS_GetResourceName() API function */ + status = OS_GetResourceName(mut_0, ResourceName, 0); + UtAssert_True(status == OS_INVALID_POINTER, "OS_GetResourceName (%lx,%ld) == OS_INVALID_POINTER", (unsigned long)mut_0, (long)status); + + status = OS_GetResourceName(msgq_0, ResourceName, sizeof(ResourceName)); + UtAssert_True(status == OS_SUCCESS, "OS_GetResourceName (%lx,%ld) == OS_SUCCESS", (unsigned long)msgq_0, (long)status); + UtAssert_StrCmp(ResourceName, "q 0", "Output value correct"); + + status = OS_GetResourceName(0, ResourceName, sizeof(ResourceName)); + UtAssert_True(status == OS_ERR_INVALID_ID, "OS_GetResourceName (%lx,%ld) == OS_ERR_INVALID_ID", (unsigned long)msgq_0, (long)status); + + status = OS_GetResourceName(bin_0, ResourceName, 1); + UtAssert_True(status == OS_ERR_NAME_TOO_LONG, "OS_GetResourceName (%lx,%ld) == OS_ERR_NAME_TOO_LONG", (unsigned long)bin_0, (long)status); + + /* The OS_DeleteAllObjects() should clean up every object created here. */ + OS_DeleteAllObjects(); +} + diff --git a/src/unit-test-coverage/shared/src/coveragetest-idmap.c b/src/unit-test-coverage/shared/src/coveragetest-idmap.c index bc7d2170c..78efe727b 100644 --- a/src/unit-test-coverage/shared/src/coveragetest-idmap.c +++ b/src/unit-test-coverage/shared/src/coveragetest-idmap.c @@ -72,52 +72,6 @@ void Test_OS_ObjectIdInit(void) UtAssert_True(actual == expected, "OS_ObjectIdInit() (%ld) == %ld", (long)actual, (long)expected); } -void Test_OS_ObjectIdMapUnmap(void) -{ - /* - * Test Case For: - * int32 OS_Id_Map(uint32 idtype, uint32 idvalue, uint32 *result); - */ - uint32 objid = 0xFFFFFFFF; - int32 expected = OS_SUCCESS; - int32 actual = OS_SUCCESS; - uint32 idtype = 0; - uint32 idvalue_out = 0; - - for (idtype = 0; idtype < OS_OBJECT_TYPE_USER; ++idtype) - { - if (idtype == OS_OBJECT_TYPE_UNDEFINED) - { - expected = OS_ERR_INVALID_ID; - } - else - { - expected = OS_SUCCESS; - } - - idvalue_out = 0; - objid = 0; - actual = OS_ObjectIdMap(idtype, 1, &objid); - - /* Verify Outputs */ - UtAssert_True(actual == expected, "OS_Id_Map() (%ld) == %ld", (long)actual, (long)expected); - UtAssert_True(objid != 0, "objid (%lu) != 0", (unsigned long)objid); - - if (idtype == OS_OBJECT_TYPE_UNDEFINED) - { - objid = 0xFFFFFFFF; - } - - actual = OS_ObjectIdUnMap(objid, idtype, &idvalue_out); - - UtAssert_True(actual == expected, "OS_Id_UnMap() (%ld) == %ld", (long)actual, (long)expected); - if (expected == OS_SUCCESS) - { - UtAssert_True(idvalue_out == 1, "idvalue_out (%lu) == 1", (unsigned long)idvalue_out); - } - } -} - void Test_OS_ObjectIdConvertLock(void) { /* @@ -280,7 +234,7 @@ void Test_OS_ObjectIdToArrayIndex(void) int32 actual = ~OS_SUCCESS; /* need to get a "valid" objid for the nominal case */ - OS_ObjectIdMap(OS_OBJECT_TYPE_OS_TASK, 1, &objid); + OS_ObjectIdCompose_Impl(OS_OBJECT_TYPE_OS_TASK, 1, &objid); actual = OS_ObjectIdToArrayIndex(OS_OBJECT_TYPE_OS_TASK, objid, &local_idx); /* Verify Outputs */ @@ -365,7 +319,7 @@ void Test_OS_ObjectIdGetById(void) /* set "true" for the remainder of tests */ OS_SharedGlobalVars.Initialized = true; - OS_ObjectIdMap(OS_OBJECT_TYPE_OS_TASK, 1, &refobjid); + OS_ObjectIdCompose_Impl(OS_OBJECT_TYPE_OS_TASK, 1, &refobjid); OS_ObjectIdToArrayIndex(OS_OBJECT_TYPE_OS_TASK, refobjid, &local_idx); OS_global_task_table[local_idx].active_id = refobjid; expected = OS_SUCCESS; @@ -606,9 +560,9 @@ void Test_OS_ConvertToArrayIndex(void) UtAssert_True(local_idx1 == local_idx2, "local_idx1 (%lu) == local_idx2 (%lu)", (unsigned long)local_idx1, (unsigned long)local_idx2); - expected = OS_ERR_INCORRECT_OBJ_TYPE; + expected = OS_ERR_INVALID_ID; actual = OS_ConvertToArrayIndex(0, &local_idx2); - UtAssert_True(actual == expected, "OS_ConvertToArrayIndex() (%ld) == OS_ERR_INCORRECT_OBJ_TYPE", (long)actual); + UtAssert_True(actual == expected, "OS_ConvertToArrayIndex() (%ld) == OS_ERR_INVALID_ID", (long)actual); } void Test_OS_ForEachObject(void) @@ -620,8 +574,11 @@ void Test_OS_ForEachObject(void) uint32 objtype = OS_OBJECT_TYPE_UNDEFINED; OS_common_record_t *rptr = NULL; uint32 local_idx = 0xFFFFFFFF; + uint32 self_id; Test_OS_ObjTypeCount_t Count; + self_id = OS_TaskGetId(); + memset(&Count, 0, sizeof(Count)); while (objtype < OS_OBJECT_TYPE_USER) @@ -638,6 +595,47 @@ void Test_OS_ForEachObject(void) UtAssert_True(Count.MutexCount == 1, "OS_ForEachObject() MutexCount (%lu) == 1", (unsigned long)Count.MutexCount); UtAssert_True(Count.OtherCount == 9, "OS_ForEachObject() OtherCount (%lu) == 9", (unsigned long)Count.OtherCount); + OS_ForEachObjectOfType(OS_OBJECT_TYPE_OS_QUEUE, self_id, ObjTypeCounter, &Count); + UtAssert_True(Count.TaskCount == 1, "OS_ForEachObjectOfType(), creator %08lx TaskCount (%lu) == 1", (unsigned long)self_id, (unsigned long)Count.TaskCount); + UtAssert_True(Count.QueueCount == 2, "OS_ForEachObjectOfType() QueueCount (%lu) == 2", (unsigned long)Count.QueueCount); + UtAssert_True(Count.MutexCount == 1, "OS_ForEachObjectOfType() MutexCount (%lu) == 1", (unsigned long)Count.MutexCount); + + OS_ForEachObjectOfType(OS_OBJECT_TYPE_OS_QUEUE, self_id ^ 1, ObjTypeCounter, &Count); + UtAssert_True(Count.TaskCount == 1, "OS_ForEachObjectOfType(), non-matching creator TaskCount (%lu) == 1", (unsigned long)Count.TaskCount); + UtAssert_True(Count.QueueCount == 2, "OS_ForEachObjectOfType() QueueCount (%lu) == 2", (unsigned long)Count.QueueCount); + UtAssert_True(Count.MutexCount == 1, "OS_ForEachObjectOfType() MutexCount (%lu) == 1", (unsigned long)Count.MutexCount); +} + +void Test_OS_GetResourceName(void) +{ + /* + * Test Case For: + * int32 OS_GetResourceName(uint32 id, char *buffer, uint32 buffer_size) + */ + uint32 local_idx = 0xFFFFFFFF; + OS_common_record_t *rptr = NULL; + char NameBuffer[OS_MAX_API_NAME]; + int32 expected; + int32 actual; + + /* + * Set up for the OS_GetResourceName function to return success + */ + /* Need a valid ID to work with */ + OS_ObjectIdFindNext(OS_OBJECT_TYPE_OS_TASK, &local_idx, &rptr); + rptr->name_entry = "UTTask"; + expected = OS_SUCCESS; + actual = OS_GetResourceName(rptr->active_id, NameBuffer, sizeof(NameBuffer)); + UtAssert_True(actual == expected, "OS_GetResourceName() (%ld) == OS_SUCCESS", (long)actual); + UtAssert_True(strcmp(NameBuffer, "UTTask") == 0, "NameBuffer (%s) == UTTask", NameBuffer); + + expected = OS_ERR_NAME_TOO_LONG; + actual = OS_GetResourceName(rptr->active_id, NameBuffer, 2); + UtAssert_True(actual == expected, "OS_GetResourceName() (%ld) == OS_ERR_NAME_TOO_LONG", (long)actual); + + expected = OS_INVALID_POINTER; + actual = OS_GetResourceName(rptr->active_id, NULL, 0); + UtAssert_True(actual == expected, "OS_GetResourceName() (%ld) == OS_INVALID_POINTER", (long)actual); } /* Osapi_Test_Setup @@ -677,7 +675,6 @@ void Osapi_Test_Teardown(void) void UtTest_Setup(void) { ADD_TEST(OS_ObjectIdInit); - ADD_TEST(OS_ObjectIdMapUnmap); ADD_TEST(OS_ObjectIdFindNext); ADD_TEST(OS_ObjectIdToArrayIndex); ADD_TEST(OS_ObjectIdFindByName); @@ -689,6 +686,7 @@ void UtTest_Setup(void) ADD_TEST(OS_ForEachObject); ADD_TEST(OS_GetMaxForObjectType); ADD_TEST(OS_GetBaseForObjectType); + ADD_TEST(OS_GetResourceName); } diff --git a/src/ut-stubs/osapi-utstub-idmap.c b/src/ut-stubs/osapi-utstub-idmap.c index d24a66308..4c4a8c958 100644 --- a/src/ut-stubs/osapi-utstub-idmap.c +++ b/src/ut-stubs/osapi-utstub-idmap.c @@ -391,6 +391,33 @@ int32 OS_ObjectIdAllocateNew(uint32 idtype, const char *name, uint32 *array_inde return Status; } +/*-------------------------------------------------------------------------------------- + Name: OS_GetResourceName + + Purpose: Stub function for OS_GetResourceName, returns either the test-supplied string + or an empty string. + + returns: status +---------------------------------------------------------------------------------------*/ +int32 OS_GetResourceName(uint32 id, char *buffer, uint32 buffer_size) +{ + int32 return_code; + + return_code = UT_DEFAULT_IMPL(OS_GetResourceName); + + if (return_code == OS_SUCCESS) + { + if (buffer_size > 0 && + UT_Stub_CopyToLocal(UT_KEY(OS_GetResourceName), buffer, buffer_size) == 0) + { + /* return an empty string by default */ + buffer[0] = 0; + } + } + + return return_code; +} + /*-------------------------------------------------------------------------------------- Name: OS_ConvertToArrayIndex @@ -429,6 +456,36 @@ int32 OS_ConvertToArrayIndex(uint32 object_id, uint32 *ArrayIndex) return return_code; } /* end OS_ConvertToArrayIndex */ +/*-------------------------------------------------------------------------------------- + Name: OS_ForEachObjectOfType + + Purpose: Stub function for OS_ForEachObjectOfType + + returns: None +---------------------------------------------------------------------------------------*/ +void OS_ForEachObjectOfType (uint32 objtype, uint32 creator_id, OS_ArgCallback_t callback_ptr, void *callback_arg) +{ + uint32 NextId; + uint32 IdSize; + OS_U32ValueWrapper_t wrapper; + + wrapper.arg_callback_func = callback_ptr; + + /* Although this is "void", Invoke the default impl to log it and invoke any hooks */ + UT_Stub_RegisterContext(UT_KEY(OS_ForEachObjectOfType), wrapper.opaque_arg); + UT_Stub_RegisterContext(UT_KEY(OS_ForEachObjectOfType), callback_arg); + UT_DEFAULT_IMPL(OS_ForEachObjectOfType); + + while (1) + { + IdSize = UT_Stub_CopyToLocal(UT_KEY(OS_ForEachObjectOfType), &NextId, sizeof(NextId)); + if (IdSize < sizeof(NextId)) + { + break; + } + (*callback_ptr)(NextId, callback_arg); + } +} /*-------------------------------------------------------------------------------------- Name: OS_ForEachOject From 084dd75e03a5b5c8726711a833cd8170dfd3f28b Mon Sep 17 00:00:00 2001 From: Joseph Hickey Date: Tue, 19 May 2020 10:06:57 -0400 Subject: [PATCH 13/20] Fix #470, bin sem unlock after task delete Corrects issue when a task waiting on a binary semaphore is deleted, it left the mutex in a locked state preventing other tasks from using the mutex. --- src/os/posix/src/os-impl-binsem.c | 54 +++++++++++++++++++++++++-- src/tests/bin-sem-test/bin-sem-test.c | 17 +++++++++ 2 files changed, 67 insertions(+), 4 deletions(-) diff --git a/src/os/posix/src/os-impl-binsem.c b/src/os/posix/src/os-impl-binsem.c index 28b01f309..b2e1832e2 100644 --- a/src/os/posix/src/os-impl-binsem.c +++ b/src/os/posix/src/os-impl-binsem.c @@ -28,9 +28,38 @@ #include "os-shared-binsem.h" #include "os-impl-binsem.h" +/* + * This controls the maximum time the that the calling thread will wait to + * acquire the condition mutex before returning an error. + * + * Under normal conditions, this lock is held by giving/taking threads very + * briefly, so the lock should be available with minimal delay. However, + * if the "taking" thread is canceled or exits abnormally without releasing the + * lock, it means any other task accessing the sem can get blocked indefinitely. + * + * There should be no reason for a user to configure this, as it should + * not be relevant in a normally operating system. This only prevents a + * deadlock condition in off-nominal circumstances. + */ +static const struct timespec OS_POSIX_BINSEM_MAX_WAIT = +{ + .tv_sec = 2, + .tv_nsec = 0 +}; + + /* Tables where the OS object information is stored */ OS_impl_binsem_internal_record_t OS_impl_bin_sem_table [OS_MAX_BIN_SEMAPHORES]; +/*--------------------------------------------------------------------------------------- + * Helper function for releasing the mutex in case the thread + * executing pthread_condwait() is canceled. + ----------------------------------------------------------------------------------------*/ +void OS_Posix_BinSemReleaseMutex(void *mut) +{ + pthread_mutex_unlock(mut); +} + /**************************************************************************************** BINARY SEMAPHORE API ***************************************************************************************/ @@ -244,10 +273,13 @@ int32 OS_BinSemGive_Impl ( uint32 sem_id ) * alternative of having a BinSemGive not wake up the other thread is a bigger issue. * * Counting sems do not suffer from this, as there is a native POSIX mechanism for those. + * + * Note: This lock should be readily available, with only minimal delay if any. + * If a long delay occurs here, it means something is fundamentally wrong. */ /* Lock the mutex ( not the table! ) */ - if ( pthread_mutex_lock(&(sem->id)) != 0 ) + if ( pthread_mutex_timedlock(&sem->id, &OS_POSIX_BINSEM_MAX_WAIT) != 0 ) { return(OS_SEM_FAILURE); } @@ -279,7 +311,7 @@ int32 OS_BinSemFlush_Impl (uint32 sem_id) sem = &OS_impl_bin_sem_table[sem_id]; /* Lock the mutex ( not the table! ) */ - if ( pthread_mutex_lock(&(sem->id)) != 0 ) + if ( pthread_mutex_timedlock(&sem->id, &OS_POSIX_BINSEM_MAX_WAIT) != 0 ) { return(OS_SEM_FAILURE); } @@ -311,12 +343,21 @@ static int32 OS_GenericBinSemTake_Impl (OS_impl_binsem_internal_record_t *sem, c sig_atomic_t flush_count; int32 return_code; + /* + * Note - this lock should be quickly available - should not delay here. + * The main delay is in the pthread_cond_wait() below. + */ /* Lock the mutex ( not the table! ) */ - if ( pthread_mutex_lock(&(sem->id)) != 0 ) + if ( pthread_mutex_timedlock(&sem->id, &OS_POSIX_BINSEM_MAX_WAIT) != 0 ) { return(OS_SEM_FAILURE); } + /* because pthread_cond_wait() is also a cancellation point, + * this uses a cleanup handler to ensure that if canceled during this call, + * the mutex is also released */ + pthread_cleanup_push(OS_Posix_BinSemReleaseMutex, &sem->id); + return_code = OS_SUCCESS; /* @@ -362,7 +403,12 @@ static int32 OS_GenericBinSemTake_Impl (OS_impl_binsem_internal_record_t *sem, c sem->current_value = 0; } - pthread_mutex_unlock(&(sem->id)); + /* + * Pop the cleanup handler. + * Passing "true" means it will be executed, which + * handles releasing the mutex. + */ + pthread_cleanup_pop(true); return return_code; } /* end OS_GenericBinSemTake_Impl */ diff --git a/src/tests/bin-sem-test/bin-sem-test.c b/src/tests/bin-sem-test/bin-sem-test.c index 029437bc7..f602fc832 100644 --- a/src/tests/bin-sem-test/bin-sem-test.c +++ b/src/tests/bin-sem-test/bin-sem-test.c @@ -137,10 +137,27 @@ void task_1(void) void BinSemCheck(void) { uint32 status; + OS_bin_sem_prop_t bin_sem_prop; + + /* Delete the task, which should be pending in OS_BinSemTake() */ + status = OS_TaskDelete(task_1_id); + UtAssert_True(status == OS_SUCCESS, "OS_TaskDelete Rc=%d", (int)status); status = OS_TimerDelete(timer_id); UtAssert_True(status == OS_SUCCESS, "OS_TimerDelete Rc=%d", (int)status); + OS_TaskDelay(100); + + /* Confirm that the semaphore itself is still operational after task deletion */ + status = OS_BinSemGive(bin_sem_id); + UtAssert_True(status == OS_SUCCESS, "BinSem give Rc=%d", (int)status); + status = OS_BinSemGetInfo (bin_sem_id, &bin_sem_prop); + UtAssert_True(status == OS_SUCCESS, "BinSem value=%d Rc=%d", (int)bin_sem_prop.value, (int)status); + status = OS_BinSemTake(bin_sem_id); + UtAssert_True(status == OS_SUCCESS, "BinSem take Rc=%d", (int)status); + status = OS_BinSemDelete(bin_sem_id); + UtAssert_True(status == OS_SUCCESS, "BinSem delete Rc=%d", (int)status); + UtAssert_True(counter < timer_counter, "Task counter (%d) < timer counter (%d)", (int)counter, (int)timer_counter); UtAssert_True(task_1_failures == 0, "Task 1 failures = %u", (unsigned int)task_1_failures); UtAssert_True(timer_failures == 0, "Timer failures = %u", (unsigned int)timer_failures); From eda62103058f9a8ac51ac0ac2f97cc0d75f85cdf Mon Sep 17 00:00:00 2001 From: Joseph Hickey Date: Tue, 19 May 2020 17:11:56 -0400 Subject: [PATCH 14/20] Fix #474, Add timecb global mutex The mutex to protect the timer callback (timecb) resource table was missing. --- src/os/posix/src/os-impl-idmap.c | 2 ++ src/os/rtems/src/os-impl-idmap.c | 2 ++ src/os/vxworks/src/os-impl-idmap.c | 2 ++ 3 files changed, 6 insertions(+) diff --git a/src/os/posix/src/os-impl-idmap.c b/src/os/posix/src/os-impl-idmap.c index f94ba8074..670477322 100644 --- a/src/os/posix/src/os-impl-idmap.c +++ b/src/os/posix/src/os-impl-idmap.c @@ -42,6 +42,7 @@ static POSIX_GlobalLock_t OS_count_sem_table_mut; static POSIX_GlobalLock_t OS_stream_table_mut; static POSIX_GlobalLock_t OS_dir_table_mut; static POSIX_GlobalLock_t OS_timebase_table_mut; +static POSIX_GlobalLock_t OS_timecb_table_mut; static POSIX_GlobalLock_t OS_module_table_mut; static POSIX_GlobalLock_t OS_filesys_table_mut; static POSIX_GlobalLock_t OS_console_mut; @@ -57,6 +58,7 @@ static POSIX_GlobalLock_t * const MUTEX_TABLE[] = [OS_OBJECT_TYPE_OS_STREAM] = &OS_stream_table_mut, [OS_OBJECT_TYPE_OS_DIR] = &OS_dir_table_mut, [OS_OBJECT_TYPE_OS_TIMEBASE] = &OS_timebase_table_mut, + [OS_OBJECT_TYPE_OS_TIMECB] = &OS_timecb_table_mut, [OS_OBJECT_TYPE_OS_MODULE] = &OS_module_table_mut, [OS_OBJECT_TYPE_OS_FILESYS] = &OS_filesys_table_mut, [OS_OBJECT_TYPE_OS_CONSOLE] = &OS_console_mut, diff --git a/src/os/rtems/src/os-impl-idmap.c b/src/os/rtems/src/os-impl-idmap.c index 0ade27ee9..32cc76441 100644 --- a/src/os/rtems/src/os-impl-idmap.c +++ b/src/os/rtems/src/os-impl-idmap.c @@ -45,6 +45,7 @@ rtems_id OS_count_sem_table_sem; rtems_id OS_stream_table_mut; rtems_id OS_dir_table_mut; rtems_id OS_timebase_table_mut; +rtems_id OS_timecb_table_mut; rtems_id OS_module_table_mut; rtems_id OS_filesys_table_mut; rtems_id OS_console_mut; @@ -60,6 +61,7 @@ static rtems_id * const MUTEX_TABLE[] = [OS_OBJECT_TYPE_OS_STREAM] = &OS_stream_table_mut, [OS_OBJECT_TYPE_OS_DIR] = &OS_dir_table_mut, [OS_OBJECT_TYPE_OS_TIMEBASE] = &OS_timebase_table_mut, + [OS_OBJECT_TYPE_OS_TIMECB] = &OS_timecb_table_mut, [OS_OBJECT_TYPE_OS_MODULE] = &OS_module_table_mut, [OS_OBJECT_TYPE_OS_FILESYS] = &OS_filesys_table_mut, [OS_OBJECT_TYPE_OS_CONSOLE] = &OS_console_mut, diff --git a/src/os/vxworks/src/os-impl-idmap.c b/src/os/vxworks/src/os-impl-idmap.c index c61f7b936..81da10c0c 100644 --- a/src/os/vxworks/src/os-impl-idmap.c +++ b/src/os/vxworks/src/os-impl-idmap.c @@ -45,6 +45,7 @@ VX_MUTEX_SEMAPHORE(OS_count_sem_table_mut_mem); VX_MUTEX_SEMAPHORE(OS_stream_table_mut_mem); VX_MUTEX_SEMAPHORE(OS_dir_table_mut_mem); VX_MUTEX_SEMAPHORE(OS_timebase_table_mut_mem); +VX_MUTEX_SEMAPHORE(OS_timecb_table_mut_mem); VX_MUTEX_SEMAPHORE(OS_module_table_mut_mem); VX_MUTEX_SEMAPHORE(OS_filesys_table_mut_mem); VX_MUTEX_SEMAPHORE(OS_console_mut_mem); @@ -60,6 +61,7 @@ VxWorks_GlobalMutex_t VX_MUTEX_TABLE[] = [OS_OBJECT_TYPE_OS_STREAM] = { .mem = OS_stream_table_mut_mem }, [OS_OBJECT_TYPE_OS_DIR] = { .mem = OS_dir_table_mut_mem }, [OS_OBJECT_TYPE_OS_TIMEBASE] = { .mem = OS_timebase_table_mut_mem }, + [OS_OBJECT_TYPE_OS_TIMECB] = { .mem = OS_timecb_table_mut_mem }, [OS_OBJECT_TYPE_OS_MODULE] = { .mem = OS_module_table_mut_mem }, [OS_OBJECT_TYPE_OS_FILESYS] = { .mem = OS_filesys_table_mut_mem }, [OS_OBJECT_TYPE_OS_CONSOLE] = { .mem = OS_console_mut_mem }, From 4ef9c1803ff5b48a34828f9525b93d1a2ea7e2c7 Mon Sep 17 00:00:00 2001 From: Joseph Hickey Date: Tue, 19 May 2020 18:16:11 -0400 Subject: [PATCH 15/20] Fix #476, add global lock/unlock wrapper Add a wrapper at the shared layer to provide a common location to check the status of global lock/unlock ops. All calls to OS_Lock_Global_Impl and OS_Unlock_Global_Impl from the shared modules are replaced with calls to this wrapper. --- src/os/posix/src/os-impl-idmap.c | 9 +- src/os/shared/inc/os-shared-idmap.h | 23 ++- src/os/shared/src/osapi-binsem.c | 4 +- src/os/shared/src/osapi-countsem.c | 4 +- src/os/shared/src/osapi-dir.c | 4 +- src/os/shared/src/osapi-file.c | 20 +-- src/os/shared/src/osapi-filesys.c | 22 +-- src/os/shared/src/osapi-idmap.c | 169 +++++++++++++++--- src/os/shared/src/osapi-module.c | 8 +- src/os/shared/src/osapi-mutex.c | 4 +- src/os/shared/src/osapi-printf.c | 2 +- src/os/shared/src/osapi-queue.c | 4 +- src/os/shared/src/osapi-sockets.c | 14 +- src/os/shared/src/osapi-task.c | 16 +- src/os/shared/src/osapi-time.c | 6 +- src/os/shared/src/osapi-timebase.c | 8 +- .../shared/src/coveragetest-idmap.c | 25 +++ src/ut-stubs/osapi-utstub-idmap.c | 10 ++ 18 files changed, 264 insertions(+), 88 deletions(-) diff --git a/src/os/posix/src/os-impl-idmap.c b/src/os/posix/src/os-impl-idmap.c index f94ba8074..758350f18 100644 --- a/src/os/posix/src/os-impl-idmap.c +++ b/src/os/posix/src/os-impl-idmap.c @@ -80,14 +80,7 @@ int32 OS_Lock_Global_Impl(uint32 idtype) POSIX_GlobalLock_t *mut; sigset_t previous; - if (idtype < MUTEX_TABLE_SIZE) - { - mut = MUTEX_TABLE[idtype]; - } - else - { - mut = NULL; - } + mut = MUTEX_TABLE[idtype]; if (mut == NULL) { diff --git a/src/os/shared/inc/os-shared-idmap.h b/src/os/shared/inc/os-shared-idmap.h index 402cf0a46..be20ad312 100644 --- a/src/os/shared/inc/os-shared-idmap.h +++ b/src/os/shared/inc/os-shared-idmap.h @@ -102,7 +102,16 @@ int32 OS_ObjectIdInit (void); /*---------------------------------------------------------------- - Function: OS_Lock_Global_Impl + Function: OS_Lock_Global + + Purpose: Locks the global table identified by "idtype" + + Returns: OS_SUCCESS on success, or relevant error code + ------------------------------------------------------------------*/ +void OS_Lock_Global(uint32 idtype); + +/*---------------------------------------------------------------- + Function: OS_Lock_Global Purpose: Locks the global table identified by "idtype" @@ -111,7 +120,17 @@ int32 OS_ObjectIdInit (void); int32 OS_Lock_Global_Impl(uint32 idtype); /*---------------------------------------------------------------- - Function: OS_Unlock_Global_Impl + Function: OS_Unlock_Global + + Purpose: Unlocks the global table identified by "idtype" + + Returns: OS_SUCCESS on success, or relevant error code + ------------------------------------------------------------------*/ +void OS_Unlock_Global(uint32 idtype); + + +/*---------------------------------------------------------------- + Function: OS_Unlock_Global Purpose: Unlocks the global table identified by "idtype" diff --git a/src/os/shared/src/osapi-binsem.c b/src/os/shared/src/osapi-binsem.c index 51b93c3e7..b98057dd3 100644 --- a/src/os/shared/src/osapi-binsem.c +++ b/src/os/shared/src/osapi-binsem.c @@ -157,7 +157,7 @@ int32 OS_BinSemDelete (uint32 sem_id) record->active_id = 0; } - OS_Unlock_Global_Impl(LOCAL_OBJID_TYPE); + OS_Unlock_Global(LOCAL_OBJID_TYPE); } return return_code; @@ -318,7 +318,7 @@ int32 OS_BinSemGetInfo (uint32 sem_id, OS_bin_sem_prop_t *bin_prop) strncpy(bin_prop->name, record->name_entry, OS_MAX_API_NAME - 1); bin_prop->creator = record->creator; return_code = OS_BinSemGetInfo_Impl (local_id, bin_prop); - OS_Unlock_Global_Impl(LOCAL_OBJID_TYPE); + OS_Unlock_Global(LOCAL_OBJID_TYPE); } return return_code; diff --git a/src/os/shared/src/osapi-countsem.c b/src/os/shared/src/osapi-countsem.c index f8cb956c6..b6b39218d 100644 --- a/src/os/shared/src/osapi-countsem.c +++ b/src/os/shared/src/osapi-countsem.c @@ -150,7 +150,7 @@ int32 OS_CountSemDelete (uint32 sem_id) record->active_id = 0; } - OS_Unlock_Global_Impl(LOCAL_OBJID_TYPE); + OS_Unlock_Global(LOCAL_OBJID_TYPE); } return return_code; @@ -288,7 +288,7 @@ int32 OS_CountSemGetInfo (uint32 sem_id, OS_count_sem_prop_t *count_prop) count_prop->creator = record->creator; return_code = OS_CountSemGetInfo_Impl (local_id, count_prop); - OS_Unlock_Global_Impl(LOCAL_OBJID_TYPE); + OS_Unlock_Global(LOCAL_OBJID_TYPE); } return return_code; diff --git a/src/os/shared/src/osapi-dir.c b/src/os/shared/src/osapi-dir.c index 7b5e04adc..4d5fe25ec 100644 --- a/src/os/shared/src/osapi-dir.c +++ b/src/os/shared/src/osapi-dir.c @@ -187,7 +187,7 @@ int32 OS_DirectoryClose(uint32 dir_id) record->active_id = 0; } - OS_Unlock_Global_Impl(LOCAL_OBJID_TYPE); + OS_Unlock_Global(LOCAL_OBJID_TYPE); } return return_code; @@ -229,7 +229,7 @@ int32 OS_DirectoryRead(uint32 dir_id, os_dirent_t *dirent) */ return_code = OS_DirRead_Impl(local_id, dirent); - OS_Unlock_Global_Impl(LOCAL_OBJID_TYPE); + OS_Unlock_Global(LOCAL_OBJID_TYPE); } return return_code; diff --git a/src/os/shared/src/osapi-file.c b/src/os/shared/src/osapi-file.c index 509744240..d64874ba9 100644 --- a/src/os/shared/src/osapi-file.c +++ b/src/os/shared/src/osapi-file.c @@ -223,7 +223,7 @@ int32 OS_close (uint32 filedes) record->active_id = 0; } - OS_Unlock_Global_Impl(LOCAL_OBJID_TYPE); + OS_Unlock_Global(LOCAL_OBJID_TYPE); } return return_code; @@ -454,7 +454,7 @@ int32 OS_rename (const char *old, const char *new) if (return_code == OS_SUCCESS) { - OS_Lock_Global_Impl(LOCAL_OBJID_TYPE); + OS_Lock_Global(LOCAL_OBJID_TYPE); for ( i =0; i < OS_MAX_NUM_OPEN_FILES; i++) { if (OS_global_stream_table[i].active_id != 0 && @@ -464,7 +464,7 @@ int32 OS_rename (const char *old, const char *new) strcpy (OS_stream_table[i].stream_name, new); } } - OS_Unlock_Global_Impl(LOCAL_OBJID_TYPE); + OS_Unlock_Global(LOCAL_OBJID_TYPE); } return return_code; @@ -609,7 +609,7 @@ int32 OS_FDGetInfo (uint32 filedes, OS_file_prop_t *fd_prop) strncpy(fd_prop->Path, record->name_entry, OS_MAX_PATH_LEN - 1); fd_prop->User = record->creator; fd_prop->IsValid = true; - OS_Unlock_Global_Impl(LOCAL_OBJID_TYPE); + OS_Unlock_Global(LOCAL_OBJID_TYPE); } return return_code; @@ -637,7 +637,7 @@ int32 OS_FileOpenCheck(const char *Filename) return_code = OS_ERROR; - OS_Lock_Global_Impl(LOCAL_OBJID_TYPE); + OS_Lock_Global(LOCAL_OBJID_TYPE); for ( i = 0; i < OS_MAX_NUM_OPEN_FILES; i++) { @@ -650,7 +650,7 @@ int32 OS_FileOpenCheck(const char *Filename) } }/* end for */ - OS_Unlock_Global_Impl(LOCAL_OBJID_TYPE); + OS_Unlock_Global(LOCAL_OBJID_TYPE); return return_code; } /* end OS_FileOpenCheck */ @@ -678,7 +678,7 @@ int32 OS_CloseFileByName(const char *Filename) return_code = OS_FS_ERR_PATH_INVALID; - OS_Lock_Global_Impl(LOCAL_OBJID_TYPE); + OS_Lock_Global(LOCAL_OBJID_TYPE); for ( i = 0; i < OS_MAX_NUM_OPEN_FILES; i++) { @@ -698,7 +698,7 @@ int32 OS_CloseFileByName(const char *Filename) } }/* end for */ - OS_Unlock_Global_Impl(LOCAL_OBJID_TYPE); + OS_Unlock_Global(LOCAL_OBJID_TYPE); return (return_code); @@ -721,7 +721,7 @@ int32 OS_CloseAllFiles(void) return_code = OS_SUCCESS; - OS_Lock_Global_Impl(LOCAL_OBJID_TYPE); + OS_Lock_Global(LOCAL_OBJID_TYPE); for ( i = 0; i < OS_MAX_NUM_OPEN_FILES; i++) { @@ -739,7 +739,7 @@ int32 OS_CloseAllFiles(void) } }/* end for */ - OS_Unlock_Global_Impl(LOCAL_OBJID_TYPE); + OS_Unlock_Global(LOCAL_OBJID_TYPE); return (return_code); diff --git a/src/os/shared/src/osapi-filesys.c b/src/os/shared/src/osapi-filesys.c index c2638ad70..aec6beb90 100644 --- a/src/os/shared/src/osapi-filesys.c +++ b/src/os/shared/src/osapi-filesys.c @@ -576,7 +576,7 @@ int32 OS_rmfs (const char *devname) global->active_id = 0; } - OS_Unlock_Global_Impl(LOCAL_OBJID_TYPE); + OS_Unlock_Global(LOCAL_OBJID_TYPE); } else { @@ -685,7 +685,7 @@ int32 OS_mount (const char *devname, const char* mountpoint) strcpy(local->virtual_mountpt, mountpoint); } - OS_Unlock_Global_Impl(LOCAL_OBJID_TYPE); + OS_Unlock_Global(LOCAL_OBJID_TYPE); } @@ -764,7 +764,7 @@ int32 OS_unmount (const char *mountpoint) local->flags &= ~(OS_FILESYS_FLAG_IS_MOUNTED_SYSTEM | OS_FILESYS_FLAG_IS_MOUNTED_VIRTUAL); } - OS_Unlock_Global_Impl(LOCAL_OBJID_TYPE); + OS_Unlock_Global(LOCAL_OBJID_TYPE); } @@ -811,7 +811,7 @@ int32 OS_fsBlocksFree (const char *name) return_code = OS_FileSysStatVolume_Impl(local_id, &statfs); - OS_Unlock_Global_Impl(LOCAL_OBJID_TYPE); + OS_Unlock_Global(LOCAL_OBJID_TYPE); if (return_code == OS_SUCCESS) { @@ -863,7 +863,7 @@ int32 OS_fsBytesFree (const char *name, uint64 *bytes_free) return_code = OS_FileSysStatVolume_Impl(local_id, &statfs); - OS_Unlock_Global_Impl(LOCAL_OBJID_TYPE); + OS_Unlock_Global(LOCAL_OBJID_TYPE); if (return_code == OS_SUCCESS) { @@ -974,7 +974,7 @@ int32 OS_FS_GetPhysDriveName(char * PhysDriveName, const char * MountPoint) return_code = OS_ERR_INCORRECT_OBJ_STATE; } - OS_Unlock_Global_Impl(LOCAL_OBJID_TYPE); + OS_Unlock_Global(LOCAL_OBJID_TYPE); } else { @@ -1012,7 +1012,7 @@ int32 OS_GetFsInfo(os_fsinfo_t *filesys_info) filesys_info->MaxFds = OS_MAX_NUM_OPEN_FILES; filesys_info->MaxVolumes = OS_MAX_FILE_SYSTEMS; - OS_Lock_Global_Impl(OS_OBJECT_TYPE_OS_STREAM); + OS_Lock_Global(OS_OBJECT_TYPE_OS_STREAM); for ( i = 0; i < OS_MAX_NUM_OPEN_FILES; i++ ) { @@ -1022,9 +1022,9 @@ int32 OS_GetFsInfo(os_fsinfo_t *filesys_info) } } - OS_Unlock_Global_Impl(OS_OBJECT_TYPE_OS_STREAM); + OS_Unlock_Global(OS_OBJECT_TYPE_OS_STREAM); - OS_Lock_Global_Impl(OS_OBJECT_TYPE_OS_FILESYS); + OS_Lock_Global(OS_OBJECT_TYPE_OS_FILESYS); for ( i = 0; i < NUM_TABLE_ENTRIES; i++ ) { @@ -1034,7 +1034,7 @@ int32 OS_GetFsInfo(os_fsinfo_t *filesys_info) } } - OS_Unlock_Global_Impl(OS_OBJECT_TYPE_OS_FILESYS); + OS_Unlock_Global(OS_OBJECT_TYPE_OS_FILESYS); return(OS_SUCCESS); } /* end OS_GetFsInfo */ @@ -1128,7 +1128,7 @@ int32 OS_TranslatePath(const char *VirtualPath, char *LocalPath) return_code = OS_ERR_INCORRECT_OBJ_STATE; } - OS_Unlock_Global_Impl(LOCAL_OBJID_TYPE); + OS_Unlock_Global(LOCAL_OBJID_TYPE); } if (return_code == OS_SUCCESS) diff --git a/src/os/shared/src/osapi-idmap.c b/src/os/shared/src/osapi-idmap.c index 7f2ea0097..a1c615d2e 100644 --- a/src/os/shared/src/osapi-idmap.c +++ b/src/os/shared/src/osapi-idmap.c @@ -74,8 +74,16 @@ typedef enum /* Tables where the OS object information is stored */ static OS_common_record_t OS_common_table[OS_MAX_TOTAL_RECORDS]; -/* Keep track of the last successfully-issued object ID of each type */ -static uint32 OS_last_id_issued[OS_OBJECT_TYPE_USER]; +typedef struct +{ + /* Keep track of the last successfully-issued object ID of each type */ + uint32 last_id_issued; + + /* The last task to lock/own this global table */ + uint32 table_owner; +} OS_objtype_state_t; + +OS_objtype_state_t OS_objtype_state[OS_OBJECT_TYPE_USER]; OS_common_record_t * const OS_global_task_table = &OS_common_table[OS_TASK_BASE]; @@ -108,7 +116,7 @@ OS_common_record_t * const OS_global_console_table = &OS_common_table[OS_CONS int32 OS_ObjectIdInit(void) { memset(OS_common_table, 0, sizeof(OS_common_table)); - memset(OS_last_id_issued, 0, sizeof(OS_last_id_issued)); + memset(OS_objtype_state, 0, sizeof(OS_objtype_state)); return OS_SUCCESS; } /* end OS_ObjectIdInit */ @@ -255,7 +263,7 @@ void OS_ObjectIdInitiateLock(OS_lock_mode_t lock_mode, uint32 idtype) { if (lock_mode != OS_LOCK_MODE_NONE) { - OS_Lock_Global_Impl(idtype); + OS_Lock_Global(idtype); } } /* end OS_ObjectIdInitiateLock */ @@ -378,9 +386,9 @@ int32 OS_ObjectIdConvertLock(OS_lock_mode_t lock_mode, uint32 idtype, uint32 ref break; } - OS_Unlock_Global_Impl(idtype); + OS_Unlock_Global(idtype); OS_TaskDelay_Impl(attempts); - OS_Lock_Global_Impl(idtype); + OS_Lock_Global(idtype); } /* @@ -407,7 +415,7 @@ int32 OS_ObjectIdConvertLock(OS_lock_mode_t lock_mode, uint32 idtype, uint32 ref if (return_code != OS_SUCCESS || lock_mode == OS_LOCK_MODE_REFCOUNT) { - OS_Unlock_Global_Impl(idtype); + OS_Unlock_Global(idtype); } } @@ -509,7 +517,7 @@ int32 OS_ObjectIdFindNext(uint32 idtype, uint32 *array_index, OS_common_record_t else { return_code = OS_ERR_NO_FREE_IDS; - idvalue = OS_last_id_issued[idtype] & OS_OBJECT_INDEX_MASK; + idvalue = OS_objtype_state[idtype].last_id_issued & OS_OBJECT_INDEX_MASK; } for (i = 0; i < max_id; ++i) @@ -566,6 +574,127 @@ int32 OS_ObjectIdFindNext(uint32 idtype, uint32 *array_index, OS_common_record_t ********************************************************************************* */ +/*---------------------------------------------------------------- + Function: OS_Lock_Global + + Purpose: Locks the global table identified by "idtype" + ------------------------------------------------------------------*/ +void OS_Lock_Global(uint32 idtype) +{ + int32 return_code; + uint32 self_task_id; + OS_objtype_state_t *objtype; + + if (idtype < OS_OBJECT_TYPE_USER) + { + objtype = &OS_objtype_state[idtype]; + self_task_id = OS_TaskGetId(); + + return_code = OS_Lock_Global_Impl(idtype); + if (return_code == OS_SUCCESS) + { + /* + * Track ownership of this table. It should only be owned by one + * task at a time, and this aids in recovery if the owning task is + * deleted or experiences an exception causing it to not be freed. + * + * This is done after successfully locking, so this has exclusive access + * to the state object. + */ + if (self_task_id == 0) + { + /* + * This just means the calling context is not an OSAL-created task. + * This is not necessarily an error, but it should be tracked. + * Also note that the root/initial task also does not have an ID. + */ + self_task_id = 0xFFFFFFFF; /* nonzero, but also won't alias a known task */ + } + + if (objtype->table_owner != 0) + { + /* this is almost certainly a bug */ + OS_DEBUG("ERROR: global %u acquired by task 0x%lx when already owned by task 0x%lx\n", + (unsigned int)idtype, (unsigned long)self_task_id, + (unsigned long)objtype->table_owner); + } + else + { + objtype->table_owner = self_task_id; + } + } + } + else + { + return_code = OS_ERR_INCORRECT_OBJ_TYPE; + } + + if (return_code != OS_SUCCESS) + { + OS_DEBUG("ERROR: unable to lock global %u, error=%d\n", (unsigned int)idtype, (int)return_code); + } +} + +/*---------------------------------------------------------------- + Function: OS_Unlock_Global + + Purpose: Unlocks the global table identified by "idtype" + ------------------------------------------------------------------*/ +void OS_Unlock_Global(uint32 idtype) +{ + int32 return_code; + uint32 self_task_id; + OS_objtype_state_t *objtype; + + if (idtype < OS_OBJECT_TYPE_USER) + { + objtype = &OS_objtype_state[idtype]; + self_task_id = OS_TaskGetId(); + + /* + * Un-track ownership of this table. It should only be owned by one + * task at a time, and this aids in recovery if the owning task is + * deleted or experiences an exception causing it to not be freed. + * + * This is done before unlocking, while this has exclusive access + * to the state object. + */ + if (self_task_id == 0) + { + /* + * This just means the calling context is not an OSAL-created task. + * This is not necessarily an error, but it should be tracked. + * Also note that the root/initial task also does not have an ID. + */ + self_task_id = 0xFFFFFFFF; /* nonzero, but also won't alias a known task */ + } + + if (objtype->table_owner != self_task_id) + { + /* this is almost certainly a bug */ + OS_DEBUG("ERROR: global %u released by task 0x%lx when owned by task 0x%lx\n", + (unsigned int)idtype, (unsigned long)self_task_id, + (unsigned long)objtype->table_owner); + } + else + { + objtype->table_owner = 0; + } + + return_code = OS_Unlock_Global_Impl(idtype); + } + else + { + return_code = OS_ERR_INCORRECT_OBJ_TYPE; + } + + if (return_code != OS_SUCCESS) + { + OS_DEBUG("ERROR: unable to unlock global %u, error=%d\n", (unsigned int)idtype, (int)return_code); + } +} + + /*---------------------------------------------------------------- * * Function: OS_ObjectIdToArrayIndex @@ -640,7 +769,7 @@ int32 OS_ObjectIdFinalizeNew(int32 operation_status, OS_common_record_t *record, else { /* success */ - OS_last_id_issued[idtype] = record->active_id; + OS_objtype_state[idtype].last_id_issued = record->active_id; } if (outid != NULL) @@ -650,7 +779,7 @@ int32 OS_ObjectIdFinalizeNew(int32 operation_status, OS_common_record_t *record, } /* Either way we must unlock the object type */ - OS_Unlock_Global_Impl(idtype); + OS_Unlock_Global(idtype); return operation_status; } /* end OS_ObjectIdFinalizeNew */ @@ -690,7 +819,7 @@ int32 OS_ObjectIdGetBySearch(OS_lock_mode_t lock_mode, uint32 idtype, OS_ObjectM } else if (lock_mode != OS_LOCK_MODE_NONE) { - OS_Unlock_Global_Impl(idtype); + OS_Unlock_Global(idtype); } if (record != NULL) @@ -757,7 +886,7 @@ int32 OS_ObjectIdFindByName (uint32 idtype, const char *name, uint32 *object_id) if (return_code == OS_SUCCESS) { *object_id = global->active_id; - OS_Unlock_Global_Impl(idtype); + OS_Unlock_Global(idtype); } return return_code; @@ -849,7 +978,7 @@ int32 OS_ObjectIdRefcountDecr(OS_common_record_t *record) } else { - OS_Lock_Global_Impl(idtype); + OS_Lock_Global(idtype); if (record->refcount > 0) { @@ -861,7 +990,7 @@ int32 OS_ObjectIdRefcountDecr(OS_common_record_t *record) return_code = OS_ERR_INCORRECT_OBJ_STATE; } - OS_Unlock_Global_Impl(idtype); + OS_Unlock_Global(idtype); } return return_code; @@ -912,7 +1041,7 @@ int32 OS_ObjectIdAllocateNew(uint32 idtype, const char *name, uint32 *array_inde return OS_ERR_INCORRECT_OBJ_TYPE; } - OS_Lock_Global_Impl(idtype); + OS_Lock_Global(idtype); /* * Check if an object of the same name already exits. @@ -940,7 +1069,7 @@ int32 OS_ObjectIdAllocateNew(uint32 idtype, const char *name, uint32 *array_inde * otherwise the global should stay locked so remaining initialization can be done */ if (return_code != OS_SUCCESS) { - OS_Unlock_Global_Impl(idtype); + OS_Unlock_Global(idtype); } return return_code; @@ -1000,7 +1129,7 @@ void OS_ForEachObject (uint32 creator_id, OS_ArgCallback_t callback_ptr, void *c obj_max = OS_GetMaxForObjectType(idtype); if (obj_max > 0) { - OS_Lock_Global_Impl(idtype); + OS_Lock_Global(idtype); obj_index = OS_GetBaseForObjectType(idtype); while (obj_max > 0) { @@ -1011,15 +1140,15 @@ void OS_ForEachObject (uint32 creator_id, OS_ArgCallback_t callback_ptr, void *c * Handle the object - Note that we must UN-lock before callback. * The callback function might lock again in a different manner. */ - OS_Unlock_Global_Impl(idtype); + OS_Unlock_Global(idtype); (*callback_ptr)(obj_id, callback_arg); - OS_Lock_Global_Impl(idtype); + OS_Lock_Global(idtype); } ++obj_index; --obj_max; } - OS_Unlock_Global_Impl(idtype); + OS_Unlock_Global(idtype); } } } /* end OS_ForEachObject */ diff --git a/src/os/shared/src/osapi-module.c b/src/os/shared/src/osapi-module.c index c6b4344b5..5582d44ef 100644 --- a/src/os/shared/src/osapi-module.c +++ b/src/os/shared/src/osapi-module.c @@ -289,7 +289,7 @@ int32 OS_ModuleUnload ( uint32 module_id ) } /* Unlock the global from OS_ObjectIdGetAndLock() */ - OS_Unlock_Global_Impl(LOCAL_OBJID_TYPE); + OS_Unlock_Global(LOCAL_OBJID_TYPE); } return return_code; @@ -325,7 +325,7 @@ int32 OS_ModuleInfo ( uint32 module_id, OS_module_prop_t *module_prop ) return_code = OS_ModuleGetInfo_Impl(local_id, module_prop); - OS_Unlock_Global_Impl(LOCAL_OBJID_TYPE); + OS_Unlock_Global(LOCAL_OBJID_TYPE); } return return_code; @@ -428,11 +428,11 @@ int32 OS_SymbolTableDump ( const char *filename, uint32 SizeLimit ) * underlying implementation may safely use globals for * state storage. */ - OS_Lock_Global_Impl(LOCAL_OBJID_TYPE); + OS_Lock_Global(LOCAL_OBJID_TYPE); return_code = OS_SymbolTableDump_Impl(translated_path, SizeLimit); - OS_Unlock_Global_Impl(LOCAL_OBJID_TYPE); + OS_Unlock_Global(LOCAL_OBJID_TYPE); return(return_code); diff --git a/src/os/shared/src/osapi-mutex.c b/src/os/shared/src/osapi-mutex.c index 6ca1b2305..7d6f4a949 100644 --- a/src/os/shared/src/osapi-mutex.c +++ b/src/os/shared/src/osapi-mutex.c @@ -145,7 +145,7 @@ int32 OS_MutSemDelete (uint32 sem_id) record->active_id = 0; } - OS_Unlock_Global_Impl(LOCAL_OBJID_TYPE); + OS_Unlock_Global(LOCAL_OBJID_TYPE); } return return_code; @@ -258,7 +258,7 @@ int32 OS_MutSemGetInfo (uint32 sem_id, OS_mut_sem_prop_t *mut_prop) return_code = OS_MutSemGetInfo_Impl(local_id, mut_prop); - OS_Unlock_Global_Impl(LOCAL_OBJID_TYPE); + OS_Unlock_Global(LOCAL_OBJID_TYPE); } return return_code; diff --git a/src/os/shared/src/osapi-printf.c b/src/os/shared/src/osapi-printf.c index 893164e1f..dd3141a90 100644 --- a/src/os/shared/src/osapi-printf.c +++ b/src/os/shared/src/osapi-printf.c @@ -236,7 +236,7 @@ int32 OS_ConsoleWrite(uint32 console_id, const char *Str) */ OS_ConsoleWakeup_Impl(local_id); - OS_Unlock_Global_Impl(OS_OBJECT_TYPE_OS_CONSOLE); + OS_Unlock_Global(OS_OBJECT_TYPE_OS_CONSOLE); } diff --git a/src/os/shared/src/osapi-queue.c b/src/os/shared/src/osapi-queue.c index 7ebdc93ae..18bf2e324 100644 --- a/src/os/shared/src/osapi-queue.c +++ b/src/os/shared/src/osapi-queue.c @@ -151,7 +151,7 @@ int32 OS_QueueDelete (uint32 queue_id) record->active_id = 0; } - OS_Unlock_Global_Impl(LOCAL_OBJID_TYPE); + OS_Unlock_Global(LOCAL_OBJID_TYPE); } return return_code; @@ -292,7 +292,7 @@ int32 OS_QueueGetInfo (uint32 queue_id, OS_queue_prop_t *queue_prop) * But this could be added in the future (i.e. current/max depth, msg size, etc) */ - OS_Unlock_Global_Impl(LOCAL_OBJID_TYPE); + OS_Unlock_Global(LOCAL_OBJID_TYPE); } return return_code; diff --git a/src/os/shared/src/osapi-sockets.c b/src/os/shared/src/osapi-sockets.c index 13b685012..4c2b76e32 100644 --- a/src/os/shared/src/osapi-sockets.c +++ b/src/os/shared/src/osapi-sockets.c @@ -194,7 +194,7 @@ int32 OS_SocketBind(uint32 sock_id, const OS_SockAddr_t *Addr) } } - OS_Unlock_Global_Impl(LOCAL_OBJID_TYPE); + OS_Unlock_Global(LOCAL_OBJID_TYPE); } return return_code; @@ -274,7 +274,7 @@ int32 OS_SocketAccept(uint32 sock_id, uint32 *connsock_id, OS_SockAddr_t *Addr, /* The actual accept impl is done without global table lock, only refcount lock */ return_code = OS_SocketAccept_Impl(local_id, conn_id, Addr, timeout); - OS_Lock_Global_Impl(LOCAL_OBJID_TYPE); + OS_Lock_Global(LOCAL_OBJID_TYPE); if (return_code == OS_SUCCESS) { /* Generate an entry name based on the remote address */ @@ -291,7 +291,7 @@ int32 OS_SocketAccept(uint32 sock_id, uint32 *connsock_id, OS_SockAddr_t *Addr, /* Decrement both ref counters that were increased earlier */ --record->refcount; --connrecord->refcount; - OS_Unlock_Global_Impl(LOCAL_OBJID_TYPE); + OS_Unlock_Global(LOCAL_OBJID_TYPE); } return return_code; @@ -334,20 +334,20 @@ int32 OS_SocketConnect(uint32 sock_id, const OS_SockAddr_t *Addr, int32 Timeout) { ++record->refcount; } - OS_Unlock_Global_Impl(LOCAL_OBJID_TYPE); + OS_Unlock_Global(LOCAL_OBJID_TYPE); } if (return_code == OS_SUCCESS) { return_code = OS_SocketConnect_Impl (local_id, Addr, Timeout); - OS_Lock_Global_Impl(LOCAL_OBJID_TYPE); + OS_Lock_Global(LOCAL_OBJID_TYPE); if (return_code == OS_SUCCESS) { OS_stream_table[local_id].stream_state |= OS_STREAM_STATE_CONNECTED | OS_STREAM_STATE_READABLE | OS_STREAM_STATE_WRITABLE; } --record->refcount; - OS_Unlock_Global_Impl(LOCAL_OBJID_TYPE); + OS_Unlock_Global(LOCAL_OBJID_TYPE); } @@ -488,7 +488,7 @@ int32 OS_SocketGetInfo (uint32 sock_id, OS_socket_prop_t *sock_prop) strncpy(sock_prop->name, record->name_entry, OS_MAX_API_NAME - 1); sock_prop->creator = record->creator; return_code = OS_SocketGetInfo_Impl (local_id, sock_prop); - OS_Unlock_Global_Impl(LOCAL_OBJID_TYPE); + OS_Unlock_Global(LOCAL_OBJID_TYPE); } return return_code; diff --git a/src/os/shared/src/osapi-task.c b/src/os/shared/src/osapi-task.c index d3946f9d1..514a263c1 100644 --- a/src/os/shared/src/osapi-task.c +++ b/src/os/shared/src/osapi-task.c @@ -89,7 +89,7 @@ static int32 OS_TaskPrepare(uint32 task_id, osal_task_entry *entrypt) * This ensures that the parent thread's OS_TaskCreate() call is fully completed, * and that nobody can call OS_TaskDelete() and possibly overwrite this data. */ - OS_Lock_Global_Impl(OS_OBJECT_TYPE_OS_TASK); + OS_Lock_Global(OS_OBJECT_TYPE_OS_TASK); /* * Verify that we still appear to own the table entry @@ -104,7 +104,7 @@ static int32 OS_TaskPrepare(uint32 task_id, osal_task_entry *entrypt) *entrypt = OS_task_table[local_id].entry_function_pointer; } - OS_Unlock_Global_Impl(OS_OBJECT_TYPE_OS_TASK); + OS_Unlock_Global(OS_OBJECT_TYPE_OS_TASK); } if (return_code == OS_SUCCESS) @@ -266,7 +266,7 @@ int32 OS_TaskDelete (uint32 task_id) delete_hook = NULL; } - OS_Unlock_Global_Impl(LOCAL_OBJID_TYPE); + OS_Unlock_Global(LOCAL_OBJID_TYPE); } /* @@ -300,7 +300,7 @@ void OS_TaskExit() { /* Only need to clear the ID as zero is the "unused" flag */ record->active_id = 0; - OS_Unlock_Global_Impl(LOCAL_OBJID_TYPE); + OS_Unlock_Global(LOCAL_OBJID_TYPE); } /* call the implementation */ @@ -358,7 +358,7 @@ int32 OS_TaskSetPriority (uint32 task_id, uint32 new_priority) } /* Unlock the global from OS_ObjectIdGetAndLock() */ - OS_Unlock_Global_Impl(LOCAL_OBJID_TYPE); + OS_Unlock_Global(LOCAL_OBJID_TYPE); } } @@ -475,7 +475,7 @@ int32 OS_TaskGetInfo (uint32 task_id, OS_task_prop_t *task_prop) return_code = OS_TaskGetInfo_Impl(local_id, task_prop); - OS_Unlock_Global_Impl(LOCAL_OBJID_TYPE); + OS_Unlock_Global(LOCAL_OBJID_TYPE); } return return_code; @@ -507,7 +507,7 @@ int32 OS_TaskInstallDeleteHandler(osal_task_entry function_pointer) */ OS_task_table[local_id].delete_hook_pointer = function_pointer; - OS_Unlock_Global_Impl(LOCAL_OBJID_TYPE); + OS_Unlock_Global(LOCAL_OBJID_TYPE); } return return_code; @@ -543,7 +543,7 @@ int32 OS_TaskFindIdBySystemData(uint32 *task_id, const void *sysdata, size_t sys if (return_code == OS_SUCCESS) { *task_id = record->active_id; - OS_Unlock_Global_Impl(LOCAL_OBJID_TYPE); + OS_Unlock_Global(LOCAL_OBJID_TYPE); } return return_code; diff --git a/src/os/shared/src/osapi-time.c b/src/os/shared/src/osapi-time.c index 4089e5a4c..63911e0cc 100644 --- a/src/os/shared/src/osapi-time.c +++ b/src/os/shared/src/osapi-time.c @@ -358,7 +358,7 @@ int32 OS_TimerSet(uint32 timer_id, uint32 start_time, uint32 interval_time) OS_TimeBaseUnlock_Impl(local->timebase_ref); /* Unlock the global from OS_ObjectIdCheck() */ - OS_Unlock_Global_Impl(OS_OBJECT_TYPE_OS_TIMECB); + OS_Unlock_Global(OS_OBJECT_TYPE_OS_TIMECB); } /* @@ -457,7 +457,7 @@ int32 OS_TimerDelete(uint32 timer_id) OS_TimeBaseUnlock_Impl(local->timebase_ref); - OS_Unlock_Global_Impl(OS_OBJECT_TYPE_OS_TIMECB); + OS_Unlock_Global(OS_OBJECT_TYPE_OS_TIMECB); } /* @@ -555,7 +555,7 @@ int32 OS_TimerGetInfo (uint32 timer_id, OS_timer_prop_t *timer_prop) timer_prop->interval_time = (uint32)OS_timecb_table[local_id].interval_time; timer_prop->accuracy = OS_timebase_table[OS_timecb_table[local_id].timebase_ref].accuracy_usec; - OS_Unlock_Global_Impl(OS_OBJECT_TYPE_OS_TIMECB); + OS_Unlock_Global(OS_OBJECT_TYPE_OS_TIMECB); } return return_code; diff --git a/src/os/shared/src/osapi-timebase.c b/src/os/shared/src/osapi-timebase.c index fbaa6291a..b8cbe9367 100644 --- a/src/os/shared/src/osapi-timebase.c +++ b/src/os/shared/src/osapi-timebase.c @@ -217,7 +217,7 @@ int32 OS_TimeBaseSet(uint32 timer_id, uint32 start_time, uint32 interval_time) OS_TimeBaseUnlock_Impl(local_id); - OS_Unlock_Global_Impl(OS_OBJECT_TYPE_OS_TIMEBASE); + OS_Unlock_Global(OS_OBJECT_TYPE_OS_TIMEBASE); } return return_code; @@ -260,7 +260,7 @@ int32 OS_TimeBaseDelete(uint32 timer_id) record->active_id = 0; } - OS_Unlock_Global_Impl(OS_OBJECT_TYPE_OS_TIMEBASE); + OS_Unlock_Global(OS_OBJECT_TYPE_OS_TIMEBASE); } return return_code; @@ -345,7 +345,7 @@ int32 OS_TimeBaseGetInfo (uint32 timebase_id, OS_timebase_prop_t *timebase_prop) return_code = OS_TimeBaseGetInfo_Impl(local_id, timebase_prop); - OS_Unlock_Global_Impl(LOCAL_OBJID_TYPE); + OS_Unlock_Global(LOCAL_OBJID_TYPE); } return return_code; @@ -428,7 +428,7 @@ void OS_TimeBase_CallbackThread(uint32 timebase_id) syncfunc = timebase->external_sync; spin_cycles = 0; - OS_Unlock_Global_Impl(OS_OBJECT_TYPE_OS_TIMEBASE); + OS_Unlock_Global(OS_OBJECT_TYPE_OS_TIMEBASE); while (1) { diff --git a/src/unit-test-coverage/shared/src/coveragetest-idmap.c b/src/unit-test-coverage/shared/src/coveragetest-idmap.c index bc7d2170c..8c2bebca2 100644 --- a/src/unit-test-coverage/shared/src/coveragetest-idmap.c +++ b/src/unit-test-coverage/shared/src/coveragetest-idmap.c @@ -118,6 +118,30 @@ void Test_OS_ObjectIdMapUnmap(void) } } +void Test_OS_LockUnlockGlobal(void) +{ + /* + * Test Case For: + * void OS_Lock_Global(uint32 idtype) + * void OS_Unlock_Global(uint32 idtype) + */ + + /* + * As these have no return codes, these tests + * exist to get coverage of the paths. + */ + OS_Lock_Global(OS_OBJECT_TYPE_OS_COUNTSEM); + OS_Unlock_Global(OS_OBJECT_TYPE_OS_COUNTSEM); + OS_Lock_Global(0); + OS_Unlock_Global(0); + OS_Lock_Global(55555); + OS_Unlock_Global(55555); + + UT_SetForceFail(UT_KEY(OS_TaskGetId), 0); + OS_Lock_Global(OS_OBJECT_TYPE_OS_BINSEM); + OS_Unlock_Global(OS_OBJECT_TYPE_OS_BINSEM); +} + void Test_OS_ObjectIdConvertLock(void) { /* @@ -678,6 +702,7 @@ void UtTest_Setup(void) { ADD_TEST(OS_ObjectIdInit); ADD_TEST(OS_ObjectIdMapUnmap); + ADD_TEST(OS_LockUnlockGlobal); ADD_TEST(OS_ObjectIdFindNext); ADD_TEST(OS_ObjectIdToArrayIndex); ADD_TEST(OS_ObjectIdFindByName); diff --git a/src/ut-stubs/osapi-utstub-idmap.c b/src/ut-stubs/osapi-utstub-idmap.c index d24a66308..f55fa3de2 100644 --- a/src/ut-stubs/osapi-utstub-idmap.c +++ b/src/ut-stubs/osapi-utstub-idmap.c @@ -32,6 +32,16 @@ UT_DEFAULT_STUB(OS_ObjectIdInit,(void)) +/* Lock/Unlock for global tables */ +void OS_Lock_Global(uint32 idtype) +{ + UT_DEFAULT_IMPL(OS_Lock_Global); +} +void OS_Unlock_Global(uint32 idtype) +{ + UT_DEFAULT_IMPL(OS_Unlock_Global); +} + /***************************************************************************** * * Stub function for OS_ObjectIdMap() From 7466997345221393ae7f1529af643b4a04b6c7dd Mon Sep 17 00:00:00 2001 From: Yasir Khan Date: Fri, 22 May 2020 11:33:13 -0400 Subject: [PATCH 16/20] Fix #374, Updated OS_ConvertToArrayIndex to verify output is in range between 0 and = 0 && TestArrayIndex < OS_MAX_TASKS , "0 < TestArrayIndex(%lu) <= OS_MAX_TASKS", (long)TestArrayIndex); actual = OS_ConvertToArrayIndex(queue_id, &TestArrayIndex); UtAssert_True(actual == expected , "OS_ConvertToArrayIndex() (%ld) == %ld ", (long)actual, (long)expected ); - UtAssert_True(TestArrayIndex == 1 , "TestArrayIndex(%lu) == 1", (long)TestArrayIndex); + UtAssert_True(TestArrayIndex >=0 && TestArrayIndex < OS_MAX_QUEUES , "0 < TestArrayIndex(%lu) <= OS_MAX_QUEUES", (long)TestArrayIndex); actual = OS_ConvertToArrayIndex(count_sem_id, &TestArrayIndex); UtAssert_True(actual == expected , "OS_ConvertToArrayIndex() (%ld) == %ld ", (long)actual, (long)expected ); - UtAssert_True(TestArrayIndex == 1 , "TestArrayIndex(%lu) == 1", (long)TestArrayIndex); + UtAssert_True(TestArrayIndex >= 0 && TestArrayIndex < OS_MAX_COUNT_SEMAPHORES , "0 < TestArrayIndex(%lu) <= OS_MAX_COUNT_SEMAPHORES", (long)TestArrayIndex); actual = OS_ConvertToArrayIndex(bin_sem_id, &TestArrayIndex); UtAssert_True(actual == expected , "OS_ConvertToArrayIndex() (%ld) == %ld ", (long)actual, (long)expected ); - UtAssert_True(TestArrayIndex == 2 , "TestArrayIndex(%lu) == 2", (long)TestArrayIndex); + UtAssert_True(TestArrayIndex >= 0 && TestArrayIndex < OS_MAX_BIN_SEMAPHORES , "0 < TestArrayIndex(%lu) <= OS_MAX_BIN_SEMAPHORES", (long)TestArrayIndex); - actual = OS_ConvertToArrayIndex(mutex_id1, &TestArrayIndex); + actual = OS_ConvertToArrayIndex(mutex_id1, &TestMutex1Index); UtAssert_True(actual == expected , "OS_ConvertToArrayIndex() (%ld) == %ld ", (long)actual, (long)expected ); - UtAssert_True(TestArrayIndex == 1 , "TestArrayIndex(%lu) == 1", (long)TestArrayIndex); + UtAssert_True(TestMutex1Index >= 0 && TestMutex1Index < OS_MAX_MUTEXES , "0 < TestMutex1Index(%lu) <= OS_MAX_MUTEXES", (long)TestMutex1Index); - actual = OS_ConvertToArrayIndex(mutex_id2, &TestArrayIndex); + actual = OS_ConvertToArrayIndex(mutex_id2, &TestMutex2Index); UtAssert_True(actual == expected , "OS_ConvertToArrayIndex() (%ld) == %ld ", (long)actual, (long)expected ); - UtAssert_True(TestArrayIndex == 2 , "TestArrayIndex(%lu) == 2", (long)TestArrayIndex); - - actual = OS_ConvertToArrayIndex(mutex_id3, &TestArrayIndex); - UtAssert_True(actual == expected , "OS_ConvertToArrayIndex() (%ld) == %ld ", (long)actual, (long)expected ); - UtAssert_True(TestArrayIndex == 3 , "TestArrayIndex(%lu) == 3", (long)TestArrayIndex); + UtAssert_True(TestMutex2Index >= 0 && TestMutex2Index < OS_MAX_MUTEXES , "0 < TestMutex2Index(%lu) <= OS_MAX_MUTEXES", (long)TestMutex2Index); + UtAssert_True(TestMutex1Index != TestMutex2Index , "TestMutex1Index(%lu) != TestMutex2Index(%lu)", (long)TestMutex1Index, (long)TestMutex2Index ); actual = OS_ConvertToArrayIndex(time_base_id, &TestArrayIndex); UtAssert_True(actual == expected , "OS_ConvertToArrayIndex() (%ld) == %ld ", (long)actual, (long)expected ); - UtAssert_True(TestArrayIndex == 1 , "TestArrayIndex(%lu) == 1", (long)TestArrayIndex); + UtAssert_True(TestArrayIndex >= 0 && TestArrayIndex < OS_MAX_TIMEBASES , "0 < TestArrayIndex(%lu) <= OS_MAX_TIMEBASES", (long)TestArrayIndex); /* * Test with extreme cases using invalid inputs and checking From f68486c31b45c4e9959bb1848f6f91cddd6f3d58 Mon Sep 17 00:00:00 2001 From: Yasir Khan Date: Tue, 26 May 2020 16:01:23 -0400 Subject: [PATCH 17/20] HOTFIX-374, Fixed OS_ConvertToArrayIndex failing test --- src/tests/idmap-api-test/idmap-api-test.c | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/src/tests/idmap-api-test/idmap-api-test.c b/src/tests/idmap-api-test/idmap-api-test.c index 15e876ef1..a66699a9e 100644 --- a/src/tests/idmap-api-test/idmap-api-test.c +++ b/src/tests/idmap-api-test/idmap-api-test.c @@ -1,3 +1,20 @@ +/* + * Copyright (c) 2020, United States government as represented by the + * administrator of the National Aeronautics Space Administration. + * All rights reserved. This software was created at NASA Goddard + * Space Flight Center pursuant to government contracts. + * + * This is governed by the NASA Open Source Agreement and may be used, + * distributed and modified only according to the terms of that agreement. + */ + +/* + * Filename: idmap-api-test.c + * + * Purpose: This file contains functional tests for "osapi-idmap" + * + */ + #include #include #include @@ -196,11 +213,11 @@ void TestIdMapApi(void) * for an error return code */ actual = OS_ConvertToArrayIndex(0x0000, &TestArrayIndex); - expected = OS_ERR_INCORRECT_OBJ_TYPE; + expected = OS_ERR_INVALID_ID; UtAssert_True(actual == expected , "OS_ConvertToArrayIndex() (%ld) == %ld ", (long)actual, (long)expected ); actual = OS_ConvertToArrayIndex(0xFFFFFFFF, &TestArrayIndex); - expected = OS_ERR_INCORRECT_OBJ_TYPE; + expected = OS_ERR_INVALID_ID; UtAssert_True(actual == expected , "OS_ConvertToArrayIndex() (%ld) == %ld ", (long)actual, (long)expected ); /* From f768a430368b997a2ed32900157cce3f58a39fb3 Mon Sep 17 00:00:00 2001 From: Joseph Hickey Date: Thu, 28 May 2020 10:49:49 -0400 Subject: [PATCH 18/20] HOTFIX: Fix parameter name consistency Use "object_id" rather than "id" for consistency with other API calls. The difference was flagged as a warning by doxygen. --- src/os/inc/osapi-os-core.h | 2 +- src/os/shared/src/osapi-idmap.c | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/os/inc/osapi-os-core.h b/src/os/inc/osapi-os-core.h index 8afd6e923..580513ea5 100644 --- a/src/os/inc/osapi-os-core.h +++ b/src/os/inc/osapi-os-core.h @@ -301,7 +301,7 @@ void OS_ApplicationExit(int32 Status); * #OS_INVALID_POINTER if the passed-in buffer is invalid * #OS_ERR_NAME_TOO_LONG if the name will not fit in the buffer provided */ -int32 OS_GetResourceName(uint32 id, char *buffer, uint32 buffer_size); +int32 OS_GetResourceName(uint32 object_id, char *buffer, uint32 buffer_size); /*-------------------------------------------------------------------------------------*/ /** diff --git a/src/os/shared/src/osapi-idmap.c b/src/os/shared/src/osapi-idmap.c index d23254c8a..e768eac4d 100644 --- a/src/os/shared/src/osapi-idmap.c +++ b/src/os/shared/src/osapi-idmap.c @@ -1172,7 +1172,7 @@ uint32 OS_IdentifyObject (uint32 object_id) * See description in API and header file for detail * *-----------------------------------------------------------------*/ -int32 OS_GetResourceName(uint32 id, char *buffer, uint32 buffer_size) +int32 OS_GetResourceName(uint32 object_id, char *buffer, uint32 buffer_size) { uint32 idtype; OS_common_record_t *record; @@ -1193,8 +1193,8 @@ int32 OS_GetResourceName(uint32 id, char *buffer, uint32 buffer_size) */ buffer[0] = 0; - idtype = OS_ObjectIdToType_Impl(id); - return_code = OS_ObjectIdGetById(OS_LOCK_MODE_GLOBAL, idtype, id, &local_id, &record); + idtype = OS_ObjectIdToType_Impl(object_id); + return_code = OS_ObjectIdGetById(OS_LOCK_MODE_GLOBAL, idtype, object_id, &local_id, &record); if (return_code == OS_SUCCESS) { if (record->name_entry != NULL) From ce45c31917412ed5174ac7bbcde8ba3379eca9d1 Mon Sep 17 00:00:00 2001 From: Joseph Hickey Date: Thu, 28 May 2020 13:29:23 -0400 Subject: [PATCH 19/20] HOTFIX: Correct issues shown in Travis CI tests - Correct merge issue with regards to the OS_Global_Unlock function - Correct use of OS_TaskGetId() - need to use Impl variant here. --- src/os/shared/src/osapi-idmap.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/os/shared/src/osapi-idmap.c b/src/os/shared/src/osapi-idmap.c index e768eac4d..1484ad905 100644 --- a/src/os/shared/src/osapi-idmap.c +++ b/src/os/shared/src/osapi-idmap.c @@ -547,7 +547,7 @@ void OS_Lock_Global(uint32 idtype) if (idtype < OS_OBJECT_TYPE_USER) { objtype = &OS_objtype_state[idtype]; - self_task_id = OS_TaskGetId(); + self_task_id = OS_TaskGetId_Impl(); return_code = OS_Lock_Global_Impl(idtype); if (return_code == OS_SUCCESS) @@ -608,7 +608,7 @@ void OS_Unlock_Global(uint32 idtype) if (idtype < OS_OBJECT_TYPE_USER) { objtype = &OS_objtype_state[idtype]; - self_task_id = OS_TaskGetId(); + self_task_id = OS_TaskGetId_Impl(); /* * Un-track ownership of this table. It should only be owned by one @@ -1209,7 +1209,7 @@ int32 OS_GetResourceName(uint32 object_id, char *buffer, uint32 buffer_size) memcpy(buffer, record->name_entry, name_len); buffer[name_len] = 0; } - OS_Unlock_Global_Impl(idtype); + OS_Unlock_Global(idtype); } return return_code; From ffe100d048cd7542cd458fbc93316a4da25f7195 Mon Sep 17 00:00:00 2001 From: "Gerardo E. Cruz-Ortiz" Date: Fri, 29 May 2020 17:31:23 -0400 Subject: [PATCH 20/20] Increase version to 5.0.18 and update ReadMe. --- README.md | 16 ++++++++++++++++ src/os/inc/osapi-version.h | 2 +- 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index af38adb95..77024e144 100644 --- a/README.md +++ b/README.md @@ -16,6 +16,22 @@ This distribution contains: ## Version History +### Development Build: 5.0.18 + +- Add functional tests for `OS_IdentifyObject`, `OS_ConvertToArrayIndex` and `OS_ForEachObject` functions. +- Fix doxygen warnings +- Unit test cases which use `OS_statfs` and run on an `RTEMS IMFS` volume will be skipped and categorized as "NA" due to `OS_ERR_NOT_IMPLEMENTED` response, rather than a failure. +- The device_name field was using the wrong length, it should be of `OS_FS_DEV_NAME_LEN` Also correct another length check on the local path name. +- For RTEMS, will not shutdown the kernel if test abort occurs. +- Unit tests work on RTEMS without BSP preallocating ramdisks +- If `OSAL_EXT_SOURCE_DIR` cache variable is set, this location will be checked first for a BSP/OS implementation layer. +- Implement `OS_GetResourceName()` and `OS_ForEachObjectOfType()`, which are new functions that allow for additional query capabilities. No impact to current behavior as the FSW does not currently use any of these new APIs. +- A functional test enhancement to `bin-sem-test` which replicates the specific conditions for the observed bug to occur. Deletes the task calling `OS_BinSemTake()` and then attempts to use the semaphore after this. +- Employ a `pthread` "cleanup handler" to handle the situation where a task is canceled during the `pthread_cond_wait()` call. This ensures that the `mutex` is unlocked as part of the cleanup, so other tasks may continue using the semaphore. +- Change all initial `mutex` locking to be a finite "timed" wait rather than an infinite wait. In all cases, the condition variable is only held for brief periods of time and should be readily available. If a task blocks for a long time, this considers the mutex "broken" and aborts, thereby avoiding deadlock. This is a "contingency" fix in that if an exception or signal or other unknown/unhandled async event occurs that leaves the mutex permanently locked. +- Adds the mutex to protect the timer callback `timecb` resource table. +- See + ### Development Build: 5.0.17 - `OS_QueueCreate()` will return an error code if the depth parameter is larger than the configured `OS_MAX_QUEUE_DEPTH`. diff --git a/src/os/inc/osapi-version.h b/src/os/inc/osapi-version.h index 3090db9d4..2212e80f9 100644 --- a/src/os/inc/osapi-version.h +++ b/src/os/inc/osapi-version.h @@ -20,7 +20,7 @@ #define OS_MAJOR_VERSION 5 /**< @brief Major version number */ #define OS_MINOR_VERSION 0 /**< @brief Minor version number */ -#define OS_REVISION 17 /**< @brief Revision number */ +#define OS_REVISION 18 /**< @brief Revision number */ #define OS_MISSION_REV 0 /**< @brief Mission revision */ /**