From b420e8d126b4d23f2abab0dfb04b258e9681ac4a Mon Sep 17 00:00:00 2001 From: Joseph Hickey Date: Fri, 21 May 2021 15:08:11 -0400 Subject: [PATCH] Fix #1016, resolve discrepancies between module API and unit tests Ensures correlation between the unit-tests and documented return values for the OSAL module API. Adds documentation for OS_ERR_OUTPUT_TOO_LARGE and OS_ERR_NAME_TOO_LONG. These are primarily validated via coverage test, as symbol table dump is only possible on VxWorks. Move the test for OS_INVALID_POINTER to after setup, where the module ID is valid. --- src/os/inc/osapi-module.h | 32 ++++++++++++------- .../osloader-test/ut_osloader_module_test.c | 25 ++++++++++++--- .../osloader-test/ut_osloader_symtable_test.c | 5 ++- 3 files changed, 45 insertions(+), 17 deletions(-) diff --git a/src/os/inc/osapi-module.h b/src/os/inc/osapi-module.h index 25cbd14da..ed5c52c5b 100644 --- a/src/os/inc/osapi-module.h +++ b/src/os/inc/osapi-module.h @@ -133,8 +133,8 @@ typedef const struct * The static table is intended to support embedded targets that do * not have module loading capability or have it disabled. * - * @param[out] symbol_address Set to the address of the symbol - * @param[in] symbol_name Name of the symbol to look up + * @param[out] symbol_address Set to the address of the symbol @nonnull + * @param[in] symbol_name Name of the symbol to look up @nonnull * * @return Execution status, see @ref OSReturnCodes * @retval #OS_SUCCESS @copybrief OS_SUCCESS @@ -152,8 +152,8 @@ int32 OS_SymbolLookup(cpuaddr *symbol_address, const char *symbol_name); * loaded with the #OS_MODULE_FLAG_LOCAL_SYMBOLS flag. * * @param[in] module_id Module ID that should contain the symbol - * @param[out] symbol_address Set to the address of the symbol - * @param[in] symbol_name Name of the symbol to look up + * @param[out] symbol_address Set to the address of the symbol @nonnull + * @param[in] symbol_name Name of the symbol to look up @nonnull * * @return Execution status, see @ref OSReturnCodes * @retval #OS_SUCCESS @copybrief OS_SUCCESS @@ -168,14 +168,21 @@ int32 OS_ModuleSymbolLookup(osal_id_t module_id, cpuaddr *symbol_address, const * * Dumps the system symbol table to the specified filename * - * @param[in] filename File to write to + * @note Not all RTOS implementations support this API. If the underlying + * module subsystem does not provide a facility to iterate through the + * symbol table, then the #OS_ERR_NOT_IMPLEMENTED status code is returned. + * + * @param[in] filename File to write to @nonnull * @param[in] size_limit Maximum number of bytes to write * * @return Execution status, see @ref OSReturnCodes * @retval #OS_SUCCESS @copybrief OS_SUCCESS * @retval #OS_ERR_NOT_IMPLEMENTED @copybrief OS_ERR_NOT_IMPLEMENTED * @retval #OS_INVALID_POINTER if the filename argument is NULL - * @retval #OS_ERROR if the symbol table could not be read or dumped + * @retval #OS_FS_ERR_PATH_INVALID if the filename argument is not valid + * @retval #OS_ERR_NAME_TOO_LONG if any of the symbol names are too long @covtest + * @retval #OS_ERR_OUTPUT_TOO_LARGE if the size_limit was reached before completing all symbols @covtest + * @retval #OS_ERROR if an other/unspecified error occurs @covtest */ int32 OS_SymbolTableDump(const char *filename, size_t size_limit); @@ -190,17 +197,18 @@ int32 OS_SymbolTableDump(const char *filename, size_t size_limit); * and #OS_MODULE_FLAG_GLOBAL_SYMBOLS for descriptions. * * @param[out] module_id Non-zero OSAL ID corresponding to the loaded module - * @param[in] module_name Name of module - * @param[in] filename File containing the object code to load + * @param[in] module_name Name of module @nonnull + * @param[in] filename File containing the object code to load @nonnull * @param[in] flags Options for the loaded module * * @return Execution status, see @ref OSReturnCodes * @retval #OS_SUCCESS @copybrief OS_SUCCESS - * @retval #OS_ERROR if the module cannot be loaded * @retval #OS_INVALID_POINTER if one of the parameters is NULL * @retval #OS_ERR_NO_FREE_IDS if the module table is full * @retval #OS_ERR_NAME_TAKEN if the name is in use * @retval #OS_ERR_NAME_TOO_LONG if the module_name is too long + * @retval #OS_FS_ERR_PATH_INVALID if the filename argument is not valid + * @retval #OS_ERROR if an other/unspecified error occurs @covtest */ int32 OS_ModuleLoad(osal_id_t *module_id, const char *module_name, const char *filename, uint32 flags); @@ -214,7 +222,8 @@ int32 OS_ModuleLoad(osal_id_t *module_id, const char *module_name, const char *f * * @return Execution status, see @ref OSReturnCodes * @retval #OS_SUCCESS @copybrief OS_SUCCESS - * @retval #OS_ERROR if the module is invalid or cannot be unloaded + * @retval #OS_ERR_INVALID_ID if the module id invalid + * @retval #OS_ERROR if an other/unspecified error occurs @covtest */ int32 OS_ModuleUnload(osal_id_t module_id); @@ -225,12 +234,13 @@ int32 OS_ModuleUnload(osal_id_t module_id); * Returns information about the loadable module * * @param[in] module_id OSAL ID of the previously the loaded module - * @param[out] module_info Buffer to store module information + * @param[out] module_info Buffer to store module information @nonnull * * @return Execution status, see @ref OSReturnCodes * @retval #OS_SUCCESS @copybrief OS_SUCCESS * @retval #OS_ERR_INVALID_ID if the module id invalid * @retval #OS_INVALID_POINTER if the pointer to the ModuleInfo structure is invalid + * @retval #OS_ERROR if an other/unspecified error occurs @covtest */ int32 OS_ModuleInfo(osal_id_t module_id, OS_module_prop_t *module_info); /**@}*/ 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 f857e6078..aca22eefa 100644 --- a/src/unit-tests/osloader-test/ut_osloader_module_test.c +++ b/src/unit-tests/osloader-test/ut_osloader_module_test.c @@ -98,6 +98,22 @@ void UT_os_module_load_test() UT_RETVAL(OS_ModuleLoad(&module_id, "TestModule", 0, OS_MODULE_FLAG_LOCAL_SYMBOLS), OS_INVALID_POINTER); + /*-----------------------------------------------------*/ + /* Name too long */ + + memset(module_name, 'x', sizeof(module_name) - 1); + module_name[sizeof(module_name) - 1] = 0; + UT_RETVAL(OS_ModuleLoad(&module_id, module_name, UT_OS_GENERIC_MODULE_NAME1, OS_MODULE_FLAG_LOCAL_SYMBOLS), + OS_ERR_NAME_TOO_LONG); + + /*-----------------------------------------------------*/ + /* Path invalid */ + + memset(module_file_name, 'x', OS_MAX_PATH_LEN - 1); + module_file_name[OS_MAX_PATH_LEN - 1] = 0; + UT_RETVAL(OS_ModuleLoad(&module_id, "TestModule", module_file_name, OS_MODULE_FLAG_LOCAL_SYMBOLS), + OS_FS_ERR_PATH_INVALID); + /*-----------------------------------------------------*/ /* #4 No-free-IDs */ @@ -204,11 +220,6 @@ void UT_os_module_info_test() return; } - /*-----------------------------------------------------*/ - /* #1 Invalid-pointer-arg */ - - UT_RETVAL(OS_ModuleInfo(OS_OBJECT_ID_UNDEFINED, NULL), OS_INVALID_POINTER); - /*-----------------------------------------------------*/ /* #2 Invalid-ID-arg */ @@ -220,6 +231,10 @@ void UT_os_module_info_test() /* Setup */ if (UT_SETUP(OS_ModuleLoad(&module_id, "Good", UT_OS_GENERIC_MODULE_NAME2, OS_MODULE_FLAG_LOCAL_SYMBOLS))) { + /* Invalid-pointer-arg */ + UT_RETVAL(OS_ModuleInfo(module_id, NULL), OS_INVALID_POINTER); + + /* Nominal */ UT_NOMINAL(OS_ModuleInfo(module_id, &module_info)); UT_TEARDOWN(OS_ModuleUnload(module_id)); 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 c28a80ffe..b9dcad61d 100644 --- a/src/unit-tests/osloader-test/ut_osloader_symtable_test.c +++ b/src/unit-tests/osloader-test/ut_osloader_symtable_test.c @@ -187,7 +187,10 @@ void UT_os_symbol_table_dump_test() /*-----------------------------------------------------*/ /* #3 Nominal */ - UT_NOMINAL_OR_NOTIMPL(OS_SymbolTableDump(UT_OS_GENERIC_MODULE_DIR "SymbolFile.dat", 32000)); + if (UT_NOMINAL_OR_NOTIMPL(OS_SymbolTableDump(UT_OS_GENERIC_MODULE_DIR "SymbolFile.dat", 32000))) + { + UT_RETVAL(OS_SymbolTableDump(UT_OS_GENERIC_MODULE_DIR "SymbolFile.dat", 0), OS_ERR_OUTPUT_TOO_LARGE); + } } /*================================================================================*