From abebf1ce76efb00e2d7ad28dd0f6879590e13adf Mon Sep 17 00:00:00 2001 From: Alex Campbell Date: Wed, 20 Jan 2021 13:03:27 -0500 Subject: [PATCH 1/3] Fix #742, make sure all pointers are checked for null --- src/os/shared/src/osapi-binsem.c | 6 +- src/os/shared/src/osapi-clock.c | 4 +- src/os/shared/src/osapi-common.c | 9 ++ src/os/shared/src/osapi-countsem.c | 6 +- src/os/shared/src/osapi-debug.c | 4 + src/os/shared/src/osapi-dir.c | 4 +- src/os/shared/src/osapi-errors.c | 1 + src/os/shared/src/osapi-file.c | 3 + src/os/shared/src/osapi-filesys.c | 14 ++- src/os/shared/src/osapi-heap.c | 1 + src/os/shared/src/osapi-idmap.c | 89 ++++++++++++++++++- src/os/shared/src/osapi-module.c | 25 ++++-- src/os/shared/src/osapi-printf.c | 9 ++ src/os/shared/src/osapi-select.c | 6 ++ src/os/shared/src/osapi-sockets.c | 15 +++- src/os/shared/src/osapi-task.c | 10 ++- src/os/shared/src/osapi-time.c | 5 ++ src/os/shared/src/osapi-timebase.c | 4 + .../shared/src/coveragetest-idmap.c | 8 +- 19 files changed, 198 insertions(+), 25 deletions(-) diff --git a/src/os/shared/src/osapi-binsem.c b/src/os/shared/src/osapi-binsem.c index 49510ae1c..1461f8aeb 100644 --- a/src/os/shared/src/osapi-binsem.c +++ b/src/os/shared/src/osapi-binsem.c @@ -100,7 +100,7 @@ int32 OS_BinSemCreate(osal_id_t *sem_id, const char *sem_name, uint32 sem_initia OS_object_token_t token; OS_bin_sem_internal_record_t *binsem; - /* Check inputs */ + /* Check parameters */ OS_CHECK_POINTER(sem_id); OS_CHECK_APINAME(sem_name); @@ -255,7 +255,7 @@ int32 OS_BinSemGetIdByName(osal_id_t *sem_id, const char *sem_name) { int32 return_code; - /* Check inputs */ + /* Check parameters */ OS_CHECK_POINTER(sem_id); OS_CHECK_POINTER(sem_name); @@ -278,7 +278,7 @@ int32 OS_BinSemGetInfo(osal_id_t sem_id, OS_bin_sem_prop_t *bin_prop) OS_object_token_t token; int32 return_code; - /* Check inputs */ + /* Check parameters */ OS_CHECK_POINTER(bin_prop); memset(bin_prop, 0, sizeof(OS_bin_sem_prop_t)); diff --git a/src/os/shared/src/osapi-clock.c b/src/os/shared/src/osapi-clock.c index 1aec8bfc8..d5a844376 100644 --- a/src/os/shared/src/osapi-clock.c +++ b/src/os/shared/src/osapi-clock.c @@ -50,7 +50,7 @@ *-----------------------------------------------------------------*/ int32 OS_GetLocalTime(OS_time_t *time_struct) { - /* Check inputs */ + /* Check parameters */ OS_CHECK_POINTER(time_struct); return OS_GetLocalTime_Impl(time_struct); @@ -67,7 +67,7 @@ int32 OS_GetLocalTime(OS_time_t *time_struct) *-----------------------------------------------------------------*/ int32 OS_SetLocalTime(const OS_time_t *time_struct) { - /* Check inputs */ + /* Check parameters */ OS_CHECK_POINTER(time_struct); return OS_SetLocalTime_Impl(time_struct); diff --git a/src/os/shared/src/osapi-common.c b/src/os/shared/src/osapi-common.c index 62a5a433b..e2832d6cd 100644 --- a/src/os/shared/src/osapi-common.c +++ b/src/os/shared/src/osapi-common.c @@ -80,6 +80,12 @@ int32 OS_NotifyEvent(OS_Event_t event, osal_id_t object_id, void *data) { int32 status; + /* + * Check parameters + * + * Note "data" is not checked, because in certain configurations it can be validly null. + */ + if (OS_SharedGlobalVars.EventHandler != NULL) { status = OS_SharedGlobalVars.EventHandler(event, object_id, data); @@ -276,6 +282,9 @@ void OS_CleanUpObject(osal_id_t object_id, void *arg) { uint32 *ObjectCount; + /* TODO: void pointer, https://github.com/nasa/osal/issues/765 */ + //OS_CHECK_POINTER(arg); + ObjectCount = (uint32 *)arg; ++(*ObjectCount); switch (OS_IdentifyObject(object_id)) diff --git a/src/os/shared/src/osapi-countsem.c b/src/os/shared/src/osapi-countsem.c index 9afcd2850..10ec1c461 100644 --- a/src/os/shared/src/osapi-countsem.c +++ b/src/os/shared/src/osapi-countsem.c @@ -92,7 +92,7 @@ int32 OS_CountSemCreate(osal_id_t *sem_id, const char *sem_name, uint32 sem_init OS_object_token_t token; OS_count_sem_internal_record_t *countsem; - /* Check inputs */ + /* Check parameters */ OS_CHECK_POINTER(sem_id); OS_CHECK_APINAME(sem_name); @@ -224,7 +224,7 @@ int32 OS_CountSemGetIdByName(osal_id_t *sem_id, const char *sem_name) { int32 return_code; - /* Check inputs */ + /* Check parameters */ OS_CHECK_POINTER(sem_id); OS_CHECK_POINTER(sem_name); @@ -247,7 +247,7 @@ int32 OS_CountSemGetInfo(osal_id_t sem_id, OS_count_sem_prop_t *count_prop) OS_object_token_t token; int32 return_code; - /* Check inputs */ + /* Check parameters */ OS_CHECK_POINTER(count_prop); memset(count_prop, 0, sizeof(OS_count_sem_prop_t)); diff --git a/src/os/shared/src/osapi-debug.c b/src/os/shared/src/osapi-debug.c index f6eef77ac..cd1b31c9d 100644 --- a/src/os/shared/src/osapi-debug.c +++ b/src/os/shared/src/osapi-debug.c @@ -55,6 +55,10 @@ void OS_DebugPrintf(uint32 Level, const char *Func, uint32 Line, const char *For { va_list va; + /* TODO: void pointer, https://github.com/nasa/osal/issues/765 */ + //OS_CHECK_POINTER(Func); + //OS_CHECK_POINTER(Format); + if (OS_SharedGlobalVars.DebugLevel >= Level) { va_start(va, Format); diff --git a/src/os/shared/src/osapi-dir.c b/src/os/shared/src/osapi-dir.c index 06f55cdb3..7566aa606 100644 --- a/src/os/shared/src/osapi-dir.c +++ b/src/os/shared/src/osapi-dir.c @@ -118,7 +118,7 @@ int32 OS_DirectoryOpen(osal_id_t *dir_id, const char *path) OS_dir_internal_record_t *dir; int32 return_code; - /* Check inputs */ + /* Check parameters */ OS_CHECK_POINTER(dir_id); return_code = OS_TranslatePath(path, local_path); @@ -183,7 +183,7 @@ int32 OS_DirectoryRead(osal_id_t dir_id, os_dirent_t *dirent) OS_object_token_t token; int32 return_code; - /* Check inputs */ + /* Check parameters */ OS_CHECK_POINTER(dirent); /* Make sure the file descriptor is legit before using it */ diff --git a/src/os/shared/src/osapi-errors.c b/src/os/shared/src/osapi-errors.c index ddfe12f9b..c20c2bb66 100644 --- a/src/os/shared/src/osapi-errors.c +++ b/src/os/shared/src/osapi-errors.c @@ -101,6 +101,7 @@ int32 OS_GetErrorName(int32 error_num, os_err_name_t *err_name) uint32 return_code; const OS_ErrorTable_Entry_t *Error; + /* Check parameters */ OS_CHECK_POINTER(err_name); Error = OS_GLOBAL_ERROR_NAME_TABLE; diff --git a/src/os/shared/src/osapi-file.c b/src/os/shared/src/osapi-file.c index 1db28e3da..13c46d9f8 100644 --- a/src/os/shared/src/osapi-file.c +++ b/src/os/shared/src/osapi-file.c @@ -112,6 +112,7 @@ int32 OS_OpenCreate(osal_id_t *filedes, const char *path, int32 flags, int32 acc OS_object_token_t token; OS_stream_internal_record_t *stream; + /* Check parameters */ OS_CHECK_POINTER(filedes); /* @@ -630,6 +631,7 @@ int32 OS_FileOpenCheck(const char *Filename) OS_object_iter_t iter; OS_stream_internal_record_t *stream; + /* Check parameters */ OS_CHECK_POINTER(Filename); return_code = OS_ERROR; @@ -666,6 +668,7 @@ int32 OS_CloseFileByName(const char *Filename) OS_object_iter_t iter; OS_stream_internal_record_t *stream; + /* Check parameters */ OS_CHECK_POINTER(Filename); return_code = OS_FS_ERR_PATH_INVALID; diff --git a/src/os/shared/src/osapi-filesys.c b/src/os/shared/src/osapi-filesys.c index 135e4ca77..e75bcbce9 100644 --- a/src/os/shared/src/osapi-filesys.c +++ b/src/os/shared/src/osapi-filesys.c @@ -75,6 +75,9 @@ const char OS_FILESYS_RAMDISK_VOLNAME_PREFIX[] = "RAM"; *-----------------------------------------------------------------*/ bool OS_FileSysFilterFree(void *ref, const OS_object_token_t *token, const OS_common_record_t *obj) { + /* Check parameters */ + OS_CHECK_POINTER(obj); + return !OS_ObjectIdDefined(obj->active_id); } @@ -91,6 +94,10 @@ bool OS_FileSysFilterFree(void *ref, const OS_object_token_t *token, const OS_co *-----------------------------------------------------------------*/ bool OS_FileSys_FindVirtMountPoint(void *ref, const OS_object_token_t *token, const OS_common_record_t *obj) { + /* Check parameters */ + OS_CHECK_POINTER(ref); + OS_CHECK_POINTER(token); + OS_filesys_internal_record_t *filesys; const char * target = (const char *)ref; size_t mplen; @@ -126,7 +133,9 @@ int32 OS_FileSys_Initialize(char *address, const char *fsdevname, const char *fs OS_object_token_t token; /* - ** Check parameters + * Check parameters + * + * Note "address" is not checked, because in certain configurations it can be validly null. */ OS_CHECK_STRING(fsdevname, sizeof(filesys->device_name), OS_FS_ERR_PATH_TOO_LONG); OS_CHECK_STRING(fsvolname, sizeof(filesys->volume_name), OS_FS_ERR_PATH_TOO_LONG); @@ -350,6 +359,7 @@ int32 OS_rmfs(const char *devname) int32 return_code; OS_object_token_t token; + /* Check parameters */ OS_CHECK_PATHNAME(devname); return_code = OS_ObjectIdGetByName(OS_LOCK_MODE_EXCLUSIVE, LOCAL_OBJID_TYPE, devname, &token); @@ -782,7 +792,7 @@ int32 OS_TranslatePath(const char *VirtualPath, char *LocalPath) /* ** Check to see if the path pointers are NULL */ - /* Check inputs */ + /* Check parameters */ OS_CHECK_POINTER(VirtualPath); OS_CHECK_POINTER(LocalPath); diff --git a/src/os/shared/src/osapi-heap.c b/src/os/shared/src/osapi-heap.c index 15ec6750e..888d0d7ec 100644 --- a/src/os/shared/src/osapi-heap.c +++ b/src/os/shared/src/osapi-heap.c @@ -50,6 +50,7 @@ *-----------------------------------------------------------------*/ int32 OS_HeapGetInfo(OS_heap_prop_t *heap_prop) { + /* Check parameters */ OS_CHECK_POINTER(heap_prop); return OS_HeapGetInfo_Impl(heap_prop); diff --git a/src/os/shared/src/osapi-idmap.c b/src/os/shared/src/osapi-idmap.c index a8fb26c4b..b9aba03bd 100644 --- a/src/os/shared/src/osapi-idmap.c +++ b/src/os/shared/src/osapi-idmap.c @@ -243,7 +243,11 @@ uint32 OS_GetBaseForObjectType(osal_objtype_t idtype) *-----------------------------------------------------------------*/ bool OS_ForEachFilterCreator(void *ref, const OS_object_token_t *token, const OS_common_record_t *obj) { - OS_creator_filter_t *filter = ref; + /* Check parameters */ + OS_CHECK_POINTER(ref); + OS_CHECK_POINTER(obj); + + OS_creator_filter_t *filter = ref; /* * Check if the obj_id is both valid and matches @@ -263,6 +267,9 @@ bool OS_ForEachFilterCreator(void *ref, const OS_object_token_t *token, const OS *-----------------------------------------------------------------*/ int32 OS_ForEachDoCallback(osal_id_t obj_id, void *ref) { + /* Check parameters */ + OS_CHECK_POINTER(ref); + OS_creator_filter_t *filter = ref; /* Just invoke the user callback */ @@ -283,6 +290,9 @@ int32 OS_ForEachDoCallback(osal_id_t obj_id, void *ref) *-----------------------------------------------------------------*/ OS_common_record_t *OS_ObjectIdGlobalFromToken(const OS_object_token_t *token) { + /* TODO: void pointer, https://github.com/nasa/osal/issues/765 */ + //OS_CHECK_POINTER(token); + uint32 base_idx = OS_GetBaseForObjectType(token->obj_type); return &OS_common_table[base_idx + token->obj_idx]; } @@ -303,6 +313,11 @@ OS_common_record_t *OS_ObjectIdGlobalFromToken(const OS_object_token_t *token) *-----------------------------------------------------------------*/ bool OS_ObjectNameMatch(void *ref, const OS_object_token_t *token, const OS_common_record_t *obj) { + /* Check parameters */ + OS_CHECK_POINTER(ref); + OS_CHECK_POINTER(token); + OS_CHECK_POINTER(obj); + return (obj->name_entry != NULL && strcmp((const char *)ref, obj->name_entry) == 0); } /* end OS_ObjectNameMatch */ @@ -324,6 +339,9 @@ bool OS_ObjectNameMatch(void *ref, const OS_object_token_t *token, const OS_comm *-----------------------------------------------------------------*/ int32 OS_ObjectIdTransactionInit(OS_lock_mode_t lock_mode, osal_objtype_t idtype, OS_object_token_t *token) { + /* Check parameters */ + OS_CHECK_POINTER(token); + memset(token, 0, sizeof(*token)); if (OS_SharedGlobalVars.Initialized == false) @@ -372,6 +390,9 @@ int32 OS_ObjectIdTransactionInit(OS_lock_mode_t lock_mode, osal_objtype_t idtype *-----------------------------------------------------------------*/ void OS_ObjectIdTransactionCancel(OS_object_token_t *token) { + /* TODO: void pointer, https://github.com/nasa/osal/issues/765 */ + //OS_CHECK_POINTER(token); + if (token->lock_mode != OS_LOCK_MODE_NONE) { OS_Unlock_Global(token); @@ -424,6 +445,9 @@ int32 OS_ObjectIdConvertToken(OS_object_token_t *token) OS_common_record_t *obj; osal_id_t expected_id; + /* Check parameters */ + OS_CHECK_POINTER(token); + obj = OS_ObjectIdGlobalFromToken(token); expected_id = OS_ObjectIdFromToken(token); @@ -583,6 +607,13 @@ int32 OS_ObjectIdFindNextMatch(OS_ObjectMatchFunc_t MatchFunc, void *arg, OS_obj OS_common_record_t *base; OS_common_record_t *record; + /* + * Check parameters + * + * Note "arg" is not checked, because in certain configurations it can be validly null. + */ + OS_CHECK_POINTER(token); + return_code = OS_ERR_NAME_NOT_FOUND; base = &OS_common_table[OS_GetBaseForObjectType(token->obj_type)]; obj_count = OS_GetMaxForObjectType(token->obj_type); @@ -638,6 +669,9 @@ int32 OS_ObjectIdFindNextFree(OS_object_token_t *token) OS_common_record_t *obj = NULL; OS_objtype_state_t *objtype_state; + /* Check parameters */ + OS_CHECK_POINTER(token); + base_id = OS_GetBaseForObjectType(token->obj_type); max_id = OS_GetMaxForObjectType(token->obj_type); objtype_state = &OS_objtype_state[token->obj_type]; @@ -716,6 +750,9 @@ void OS_Lock_Global(OS_object_token_t *token) osal_id_t self_task_id; OS_objtype_state_t *objtype; + /* TODO: void pointer, https://github.com/nasa/osal/issues/765 */ + //OS_CHECK_POINTER(token); + if (token->obj_type < OS_OBJECT_TYPE_USER && token->lock_mode != OS_LOCK_MODE_NONE) { objtype = &OS_objtype_state[token->obj_type]; @@ -779,6 +816,9 @@ void OS_Unlock_Global(OS_object_token_t *token) { OS_objtype_state_t *objtype; + /* TODO: void pointer, https://github.com/nasa/osal/issues/765 */ + //OS_CHECK_POINTER(token); + if (token->obj_type < OS_OBJECT_TYPE_USER && token->lock_mode != OS_LOCK_MODE_NONE) { objtype = &OS_objtype_state[token->obj_type]; @@ -827,6 +867,9 @@ void OS_WaitForStateChange(OS_object_token_t *token, uint32 attempts) osal_key_t saved_unlock_key; OS_objtype_state_t *objtype; + /* TODO: void pointer, https://github.com/nasa/osal/issues/765 */ + //OS_CHECK_POINTER(token); + /* * This needs to release the lock, to allow other * tasks to make a change to the table. But to avoid @@ -876,6 +919,13 @@ int32 OS_ObjectIdFinalizeNew(int32 operation_status, OS_object_token_t *token, o { osal_id_t final_id; + /* + * Check parameters + * + * Note "outid" is not checked, because in certain configurations it can be validly null. + */ + OS_CHECK_POINTER(token); + /* if operation was unsuccessful, then clear * the active_id field within the record, so * the record can be re-used later. @@ -920,6 +970,9 @@ int32 OS_ObjectIdFinalizeDelete(int32 operation_status, OS_object_token_t *token { osal_id_t final_id; + /* Check parameters */ + OS_CHECK_POINTER(token); + /* Clear the OSAL ID if successful - this returns the record to the pool */ if (operation_status == OS_SUCCESS) { @@ -1019,6 +1072,9 @@ int32 OS_ObjectIdFindByName(osal_objtype_t idtype, const char *name, osal_id_t * int32 return_code; OS_object_token_t token; + /* Check parameters */ + OS_CHECK_POINTER(object_id); + /* * As this is an internal-only function, calling it will NULL is allowed. * This is required by the file/dir/socket API since these DO allow multiple @@ -1107,6 +1163,14 @@ void OS_ObjectIdTransactionFinish(OS_object_token_t *token, osal_id_t *final_id) { OS_common_record_t *record; + /* TODO: void pointer, https://github.com/nasa/osal/issues/765 */ + /* + * Check parameters + * + * Note "final_id" is not checked, because in certain configurations it can be validly null. + */ + //OS_CHECK_POINTER(token); + if (token->lock_mode == OS_LOCK_MODE_NONE) { /* nothing to do */ @@ -1278,6 +1342,10 @@ int32 OS_ObjectIdAllocateNew(osal_objtype_t idtype, const char *name, OS_object_ ------------------------------------------------------------------*/ void OS_ObjectIdTransferToken(OS_object_token_t *token_from, OS_object_token_t *token_to) { + /* TODO: void pointer, https://github.com/nasa/osal/issues/765 */ + //OS_CHECK_POINTER(token_to); + //OS_CHECK_POINTER(token_from); + /* start with a simple copy */ *token_to = *token_from; @@ -1297,6 +1365,13 @@ void OS_ObjectIdTransferToken(OS_object_token_t *token_from, OS_object_token_t * int32 OS_ObjectIdIteratorInit(OS_ObjectMatchFunc_t matchfunc, void *matcharg, osal_objtype_t objtype, OS_object_iter_t *iter) { + /* + * Check parameters + * + * Note "matcharg" is not checked, because in certain configurations it can be validly null. + */ + OS_CHECK_POINTER(iter); + iter->match = matchfunc; iter->arg = matcharg; iter->limit = OS_GetMaxForObjectType(objtype); @@ -1312,6 +1387,9 @@ int32 OS_ObjectIdIteratorInit(OS_ObjectMatchFunc_t matchfunc, void *matcharg, os ------------------------------------------------------------------*/ bool OS_ObjectFilterActive(void *ref, const OS_object_token_t *token, const OS_common_record_t *obj) { + /* Check parameters */ + OS_CHECK_POINTER(obj); + return OS_ObjectIdDefined(obj->active_id); } @@ -1334,6 +1412,8 @@ bool OS_ObjectIdIteratorGetNext(OS_object_iter_t *iter) { OS_common_record_t *record; bool got_next; + + OS_CHECK_POINTER(iter); got_next = false; iter->token.obj_id = OS_OBJECT_ID_UNDEFINED; @@ -1376,6 +1456,10 @@ int32 OS_ObjectIdIteratorProcessEntry(OS_object_iter_t *iter, int32 (*func)(osal { int32 status; + /* Check parameters */ + OS_CHECK_POINTER(iter); + OS_CHECK_POINTER(func); + /* * This needs to temporarily unlock the global, * call the handler function, then re-lock. @@ -1438,6 +1522,9 @@ void OS_ForEachObjectOfType(osal_objtype_t idtype, osal_id_t creator_id, OS_ArgC OS_object_iter_t iter; OS_creator_filter_t filter; + /* TODO: void pointer, https://github.com/nasa/osal/issues/765 */ + //OS_CHECK_POINTER(callback_arg) + filter.creator_id = creator_id; filter.user_callback = callback_ptr; filter.user_arg = callback_arg; diff --git a/src/os/shared/src/osapi-module.c b/src/os/shared/src/osapi-module.c index 362334115..96c5d5835 100644 --- a/src/os/shared/src/osapi-module.c +++ b/src/os/shared/src/osapi-module.c @@ -101,6 +101,14 @@ int32 OS_SymbolLookup_Static(cpuaddr *SymbolAddress, const char *SymbolName, con int32 return_code = OS_ERR_NOT_IMPLEMENTED; OS_static_symbol_record_t *StaticSym = OS_STATIC_SYMTABLE_SOURCE; + /* + * Check parameters + * + * Note "ModuleName" is not checked, because in certain configurations it can be validly null. + */ + OS_CHECK_POINTER(SymbolAddress); + OS_CHECK_POINTER(SymbolName); + while (StaticSym != NULL) { if (StaticSym->Name == NULL) @@ -139,6 +147,9 @@ int32 OS_ModuleLoad_Static(const char *ModuleName) int32 return_code = OS_ERR_NAME_NOT_FOUND; OS_static_symbol_record_t *StaticSym = OS_STATIC_SYMTABLE_SOURCE; + /* Check parameters */ + OS_CHECK_POINTER(ModuleName); + while (StaticSym != NULL) { if (StaticSym->Name == NULL) @@ -194,11 +205,11 @@ int32 OS_ModuleLoad(osal_id_t *module_id, const char *module_name, const char *f OS_module_internal_record_t *module; /* - ** Check parameters - ** - ** Note "filename" is not checked, because in certain configurations it can be validly - ** null. filename is checked for NULL-ness by the OS_TranslatePath() later. - */ + * Check parameters + * + * Note "filename" is not checked, because in certain configurations it can be validly + * null. filename is checked for NULL-ness by the OS_TranslatePath() later. + */ OS_CHECK_POINTER(module_id); OS_CHECK_APINAME(module_name); @@ -454,9 +465,7 @@ int32 OS_SymbolTableDump(const char *filename, size_t SizeLimit) char translated_path[OS_MAX_LOCAL_PATH_LEN]; OS_object_token_t token; - /* - ** Check parameters - */ + /* Check parameters */ OS_CHECK_POINTER(filename); /* diff --git a/src/os/shared/src/osapi-printf.c b/src/os/shared/src/osapi-printf.c index a174a4c61..4ed98e732 100644 --- a/src/os/shared/src/osapi-printf.c +++ b/src/os/shared/src/osapi-printf.c @@ -142,6 +142,12 @@ static int32 OS_Console_CopyOut(OS_console_internal_record_t *console, const cha size_t WriteOffset; int32 return_code; + /* Check parameters */ + OS_CHECK_POINTER(console); + OS_CHECK_POINTER(Str); + OS_CHECK_POINTER(NextWritePos); + + return_code = OS_ERROR; pmsg = Str; WriteOffset = *NextWritePos; @@ -256,6 +262,9 @@ void OS_printf(const char *String, ...) char msg_buffer[OS_BUFFER_SIZE]; int actualsz; + /* TODO: void pointer, https://github.com/nasa/osal/issues/765 */ + //OS_CHECK_POINTER(String); + if (!OS_SharedGlobalVars.Initialized) { /* diff --git a/src/os/shared/src/osapi-select.c b/src/os/shared/src/osapi-select.c index d60b03288..3158ebbc3 100644 --- a/src/os/shared/src/osapi-select.c +++ b/src/os/shared/src/osapi-select.c @@ -89,6 +89,12 @@ int32 OS_SelectMultiple(OS_FdSet *ReadSet, OS_FdSet *WriteSet, int32 msecs) { int32 return_code; + /* + * Check parameters + * + * Note "ReadSet" and "WriteSet" are not checked, because in certain configurations they can be validly null. + */ + /* * This does not currently increment any refcounts. * That means a file/socket can be closed while actively inside a diff --git a/src/os/shared/src/osapi-sockets.c b/src/os/shared/src/osapi-sockets.c index f610b8660..66f2aeae5 100644 --- a/src/os/shared/src/osapi-sockets.c +++ b/src/os/shared/src/osapi-sockets.c @@ -96,6 +96,10 @@ void OS_CreateSocketName(const OS_object_token_t *token, const OS_SockAddr_t *Ad uint16 port; OS_stream_internal_record_t *sock; + /* TODO: void pointer, https://github.com/nasa/osal/issues/765 */ + //OS_CHECK_POINTER(token); + //OS_CHECK_POINTER(Addr); + sock = OS_OBJECT_TABLE_GET(OS_stream_table, *token); if (OS_SocketAddrToString_Impl(sock->stream_name, OS_MAX_API_NAME, Addr) != OS_SUCCESS) @@ -359,7 +363,11 @@ int32 OS_SocketRecvFrom(osal_id_t sock_id, void *buffer, size_t buflen, OS_SockA OS_object_token_t token; int32 return_code; - /* Check Parameters */ + /* + * Check parameters + * + * Note "RemoteAddr" is not checked, because in certain configurations it can be validly null. + */ OS_CHECK_POINTER(buffer); OS_CHECK_SIZE(buflen); @@ -493,6 +501,7 @@ int32 OS_SocketGetInfo(osal_id_t sock_id, OS_socket_prop_t *sock_prop) *-----------------------------------------------------------------*/ int32 OS_SocketAddrInit(OS_SockAddr_t *Addr, OS_SocketDomain_t Domain) { + /* Check parameters */ OS_CHECK_POINTER(Addr); return OS_SocketAddrInit_Impl(Addr, Domain); @@ -508,6 +517,7 @@ int32 OS_SocketAddrInit(OS_SockAddr_t *Addr, OS_SocketDomain_t Domain) *-----------------------------------------------------------------*/ int32 OS_SocketAddrToString(char *buffer, size_t buflen, const OS_SockAddr_t *Addr) { + /* Check parameters */ OS_CHECK_POINTER(Addr); OS_CHECK_POINTER(buffer); OS_CHECK_SIZE(buflen); @@ -525,6 +535,7 @@ int32 OS_SocketAddrToString(char *buffer, size_t buflen, const OS_SockAddr_t *Ad *-----------------------------------------------------------------*/ int32 OS_SocketAddrFromString(OS_SockAddr_t *Addr, const char *string) { + /* Check parameters */ OS_CHECK_POINTER(Addr); OS_CHECK_POINTER(string); @@ -541,6 +552,7 @@ int32 OS_SocketAddrFromString(OS_SockAddr_t *Addr, const char *string) *-----------------------------------------------------------------*/ int32 OS_SocketAddrGetPort(uint16 *PortNum, const OS_SockAddr_t *Addr) { + /* Check parameters */ OS_CHECK_POINTER(Addr); OS_CHECK_POINTER(PortNum); @@ -557,6 +569,7 @@ int32 OS_SocketAddrGetPort(uint16 *PortNum, const OS_SockAddr_t *Addr) *-----------------------------------------------------------------*/ int32 OS_SocketAddrSetPort(OS_SockAddr_t *Addr, uint16 PortNum) { + /* Check parameters */ OS_CHECK_POINTER(Addr); return OS_SocketAddrSetPort_Impl(Addr, PortNum); diff --git a/src/os/shared/src/osapi-task.c b/src/os/shared/src/osapi-task.c index b46db2677..2cfa88aba 100644 --- a/src/os/shared/src/osapi-task.c +++ b/src/os/shared/src/osapi-task.c @@ -85,6 +85,9 @@ static int32 OS_TaskPrepare(osal_id_t task_id, osal_task_entry *entrypt) OS_object_token_t token; OS_task_internal_record_t *task; + /* Check parameters */ + OS_CHECK_POINTER(entrypt); + return_code = OS_ObjectIdGetById(OS_LOCK_MODE_GLOBAL, OS_OBJECT_TYPE_OS_TASK, task_id, &token); if (return_code == OS_SUCCESS) { @@ -178,7 +181,11 @@ int32 OS_TaskCreate(osal_id_t *task_id, const char *task_name, osal_task_entry f OS_object_token_t token; OS_task_internal_record_t *task; - /* Check for NULL pointers */ + /* + * Check parameters + * + * Note "stack_pointer" is not checked, because in certain configurations it can be validly null. + */ OS_CHECK_POINTER(task_id); OS_CHECK_POINTER(function_pointer); OS_CHECK_APINAME(task_name); @@ -381,6 +388,7 @@ int32 OS_TaskGetIdByName(osal_id_t *task_id, const char *task_name) { int32 return_code; + /* Check parameters */ OS_CHECK_POINTER(task_id); OS_CHECK_POINTER(task_name); diff --git a/src/os/shared/src/osapi-time.c b/src/os/shared/src/osapi-time.c index 11e654caa..0400873de 100644 --- a/src/os/shared/src/osapi-time.c +++ b/src/os/shared/src/osapi-time.c @@ -111,6 +111,7 @@ static int32 OS_DoTimerAdd(osal_id_t *timer_id, const char *timer_name, osal_id_ OS_CHECK_POINTER(timer_id); OS_CHECK_APINAME(timer_name); OS_CHECK_POINTER(callback_ptr); + OS_CHECK_POINTER(callback_arg); /* * Check our context. Not allowed to use the timer API from a timer callback. @@ -218,6 +219,9 @@ static void OS_Timer_NoArgCallback(osal_id_t objid, void *arg) { OS_Timer_ArgWrapper_t Conv; + /* TODO: void pointer, https://github.com/nasa/osal/issues/765 */ + //OS_CHECK_POINTER(arg); + /* * Note - did not write this as simply *((OS_SimpleCallback_t)arg) because * technically you cannot cast a void * to a function pointer. @@ -478,6 +482,7 @@ int32 OS_TimerGetIdByName(osal_id_t *timer_id, const char *timer_name) int32 return_code; osal_objtype_t objtype; + /* Check parameters */ OS_CHECK_POINTER(timer_id); OS_CHECK_POINTER(timer_name); diff --git a/src/os/shared/src/osapi-timebase.c b/src/os/shared/src/osapi-timebase.c index affca25fb..f922b2630 100644 --- a/src/os/shared/src/osapi-timebase.c +++ b/src/os/shared/src/osapi-timebase.c @@ -263,6 +263,7 @@ int32 OS_TimeBaseGetIdByName(osal_id_t *timer_id, const char *timebase_name) int32 return_code; osal_objtype_t objtype; + /* Check parameters */ OS_CHECK_POINTER(timer_id); OS_CHECK_APINAME(timebase_name); @@ -539,6 +540,9 @@ int32 OS_Milli2Ticks(uint32 milli_seconds, int *ticks) uint64 num_of_ticks; int32 return_code = OS_SUCCESS; + /* Check parameters */ + OS_CHECK_POINTER(ticks); + num_of_ticks = (((uint64)milli_seconds * OS_SharedGlobalVars.TicksPerSecond) + 999) / 1000; /* Check against maximum int32 (limit from some OS's) */ diff --git a/src/unit-test-coverage/shared/src/coveragetest-idmap.c b/src/unit-test-coverage/shared/src/coveragetest-idmap.c index 6f7a4db4f..440d852c8 100644 --- a/src/unit-test-coverage/shared/src/coveragetest-idmap.c +++ b/src/unit-test-coverage/shared/src/coveragetest-idmap.c @@ -429,10 +429,14 @@ void Test_OS_ObjectIdFindByName(void) char TaskName[] = "UT_find"; osal_id_t objid; int32 expected = OS_ERR_NAME_NOT_FOUND; - int32 actual = OS_ObjectIdFindByName(OS_OBJECT_TYPE_UNDEFINED, NULL, NULL); - + int32 actual = OS_ObjectIdFindByName(OS_OBJECT_TYPE_UNDEFINED, NULL, &objid); UtAssert_True(actual == expected, "OS_ObjectFindIdByName(%s) (%ld) == OS_ERR_NAME_NOT_FOUND", "NULL", (long)actual); + expected = OS_INVALID_POINTER; + actual = OS_ObjectIdFindByName(OS_OBJECT_TYPE_UNDEFINED, TaskName, NULL); + UtAssert_True(actual == expected, "OS_ObjectFindIdByName(%s) (%ld) == OS_INVALID_POINTER", "NULL", (long)actual); + + /* * Pass in a name that is beyond OS_MAX_API_NAME */ From 4a77997f95f74ec1960edf2344f4515bf27cbdb3 Mon Sep 17 00:00:00 2001 From: Alex Campbell Date: Fri, 22 Jan 2021 09:24:22 -0500 Subject: [PATCH 2/3] Fix #742, remove null checks from internal methods. --- src/os/shared/src/osapi-common.c | 9 -- src/os/shared/src/osapi-debug.c | 4 - src/os/shared/src/osapi-filesys.c | 15 +-- src/os/shared/src/osapi-idmap.c | 115 +++--------------- src/os/shared/src/osapi-module.c | 11 -- src/os/shared/src/osapi-printf.c | 8 +- src/os/shared/src/osapi-select.c | 8 +- src/os/shared/src/osapi-sockets.c | 12 +- src/os/shared/src/osapi-task.c | 11 +- src/os/shared/src/osapi-time.c | 8 +- src/os/shared/src/osapi-timebase.c | 3 - .../shared/src/coveragetest-idmap.c | 5 - 12 files changed, 36 insertions(+), 173 deletions(-) diff --git a/src/os/shared/src/osapi-common.c b/src/os/shared/src/osapi-common.c index e2832d6cd..62a5a433b 100644 --- a/src/os/shared/src/osapi-common.c +++ b/src/os/shared/src/osapi-common.c @@ -80,12 +80,6 @@ int32 OS_NotifyEvent(OS_Event_t event, osal_id_t object_id, void *data) { int32 status; - /* - * Check parameters - * - * Note "data" is not checked, because in certain configurations it can be validly null. - */ - if (OS_SharedGlobalVars.EventHandler != NULL) { status = OS_SharedGlobalVars.EventHandler(event, object_id, data); @@ -282,9 +276,6 @@ void OS_CleanUpObject(osal_id_t object_id, void *arg) { uint32 *ObjectCount; - /* TODO: void pointer, https://github.com/nasa/osal/issues/765 */ - //OS_CHECK_POINTER(arg); - ObjectCount = (uint32 *)arg; ++(*ObjectCount); switch (OS_IdentifyObject(object_id)) diff --git a/src/os/shared/src/osapi-debug.c b/src/os/shared/src/osapi-debug.c index cd1b31c9d..f6eef77ac 100644 --- a/src/os/shared/src/osapi-debug.c +++ b/src/os/shared/src/osapi-debug.c @@ -55,10 +55,6 @@ void OS_DebugPrintf(uint32 Level, const char *Func, uint32 Line, const char *For { va_list va; - /* TODO: void pointer, https://github.com/nasa/osal/issues/765 */ - //OS_CHECK_POINTER(Func); - //OS_CHECK_POINTER(Format); - if (OS_SharedGlobalVars.DebugLevel >= Level) { va_start(va, Format); diff --git a/src/os/shared/src/osapi-filesys.c b/src/os/shared/src/osapi-filesys.c index e75bcbce9..8d1b44c7e 100644 --- a/src/os/shared/src/osapi-filesys.c +++ b/src/os/shared/src/osapi-filesys.c @@ -75,9 +75,6 @@ const char OS_FILESYS_RAMDISK_VOLNAME_PREFIX[] = "RAM"; *-----------------------------------------------------------------*/ bool OS_FileSysFilterFree(void *ref, const OS_object_token_t *token, const OS_common_record_t *obj) { - /* Check parameters */ - OS_CHECK_POINTER(obj); - return !OS_ObjectIdDefined(obj->active_id); } @@ -94,10 +91,6 @@ bool OS_FileSysFilterFree(void *ref, const OS_object_token_t *token, const OS_co *-----------------------------------------------------------------*/ bool OS_FileSys_FindVirtMountPoint(void *ref, const OS_object_token_t *token, const OS_common_record_t *obj) { - /* Check parameters */ - OS_CHECK_POINTER(ref); - OS_CHECK_POINTER(token); - OS_filesys_internal_record_t *filesys; const char * target = (const char *)ref; size_t mplen; @@ -133,10 +126,10 @@ int32 OS_FileSys_Initialize(char *address, const char *fsdevname, const char *fs OS_object_token_t token; /* - * Check parameters - * - * Note "address" is not checked, because in certain configurations it can be validly null. - */ + * Check parameters + * + * Note "address" is not checked, because in certain configurations it can be validly null. + */ OS_CHECK_STRING(fsdevname, sizeof(filesys->device_name), OS_FS_ERR_PATH_TOO_LONG); OS_CHECK_STRING(fsvolname, sizeof(filesys->volume_name), OS_FS_ERR_PATH_TOO_LONG); diff --git a/src/os/shared/src/osapi-idmap.c b/src/os/shared/src/osapi-idmap.c index b9aba03bd..d6e16791e 100644 --- a/src/os/shared/src/osapi-idmap.c +++ b/src/os/shared/src/osapi-idmap.c @@ -91,7 +91,6 @@ typedef struct void * user_arg; } OS_creator_filter_t; - /* * Global ID storage tables */ @@ -243,11 +242,7 @@ uint32 OS_GetBaseForObjectType(osal_objtype_t idtype) *-----------------------------------------------------------------*/ bool OS_ForEachFilterCreator(void *ref, const OS_object_token_t *token, const OS_common_record_t *obj) { - /* Check parameters */ - OS_CHECK_POINTER(ref); - OS_CHECK_POINTER(obj); - - OS_creator_filter_t *filter = ref; + OS_creator_filter_t *filter = ref; /* * Check if the obj_id is both valid and matches @@ -267,9 +262,6 @@ bool OS_ForEachFilterCreator(void *ref, const OS_object_token_t *token, const OS *-----------------------------------------------------------------*/ int32 OS_ForEachDoCallback(osal_id_t obj_id, void *ref) { - /* Check parameters */ - OS_CHECK_POINTER(ref); - OS_creator_filter_t *filter = ref; /* Just invoke the user callback */ @@ -277,7 +269,6 @@ int32 OS_ForEachDoCallback(osal_id_t obj_id, void *ref) return OS_SUCCESS; } - /*---------------------------------------------------------------- * * Function: OS_ObjectIdGlobalFromToken @@ -290,9 +281,6 @@ int32 OS_ForEachDoCallback(osal_id_t obj_id, void *ref) *-----------------------------------------------------------------*/ OS_common_record_t *OS_ObjectIdGlobalFromToken(const OS_object_token_t *token) { - /* TODO: void pointer, https://github.com/nasa/osal/issues/765 */ - //OS_CHECK_POINTER(token); - uint32 base_idx = OS_GetBaseForObjectType(token->obj_type); return &OS_common_table[base_idx + token->obj_idx]; } @@ -313,11 +301,6 @@ OS_common_record_t *OS_ObjectIdGlobalFromToken(const OS_object_token_t *token) *-----------------------------------------------------------------*/ bool OS_ObjectNameMatch(void *ref, const OS_object_token_t *token, const OS_common_record_t *obj) { - /* Check parameters */ - OS_CHECK_POINTER(ref); - OS_CHECK_POINTER(token); - OS_CHECK_POINTER(obj); - return (obj->name_entry != NULL && strcmp((const char *)ref, obj->name_entry) == 0); } /* end OS_ObjectNameMatch */ @@ -339,9 +322,6 @@ bool OS_ObjectNameMatch(void *ref, const OS_object_token_t *token, const OS_comm *-----------------------------------------------------------------*/ int32 OS_ObjectIdTransactionInit(OS_lock_mode_t lock_mode, osal_objtype_t idtype, OS_object_token_t *token) { - /* Check parameters */ - OS_CHECK_POINTER(token); - memset(token, 0, sizeof(*token)); if (OS_SharedGlobalVars.Initialized == false) @@ -390,9 +370,6 @@ int32 OS_ObjectIdTransactionInit(OS_lock_mode_t lock_mode, osal_objtype_t idtype *-----------------------------------------------------------------*/ void OS_ObjectIdTransactionCancel(OS_object_token_t *token) { - /* TODO: void pointer, https://github.com/nasa/osal/issues/765 */ - //OS_CHECK_POINTER(token); - if (token->lock_mode != OS_LOCK_MODE_NONE) { OS_Unlock_Global(token); @@ -445,9 +422,6 @@ int32 OS_ObjectIdConvertToken(OS_object_token_t *token) OS_common_record_t *obj; osal_id_t expected_id; - /* Check parameters */ - OS_CHECK_POINTER(token); - obj = OS_ObjectIdGlobalFromToken(token); expected_id = OS_ObjectIdFromToken(token); @@ -607,13 +581,6 @@ int32 OS_ObjectIdFindNextMatch(OS_ObjectMatchFunc_t MatchFunc, void *arg, OS_obj OS_common_record_t *base; OS_common_record_t *record; - /* - * Check parameters - * - * Note "arg" is not checked, because in certain configurations it can be validly null. - */ - OS_CHECK_POINTER(token); - return_code = OS_ERR_NAME_NOT_FOUND; base = &OS_common_table[OS_GetBaseForObjectType(token->obj_type)]; obj_count = OS_GetMaxForObjectType(token->obj_type); @@ -669,9 +636,6 @@ int32 OS_ObjectIdFindNextFree(OS_object_token_t *token) OS_common_record_t *obj = NULL; OS_objtype_state_t *objtype_state; - /* Check parameters */ - OS_CHECK_POINTER(token); - base_id = OS_GetBaseForObjectType(token->obj_type); max_id = OS_GetMaxForObjectType(token->obj_type); objtype_state = &OS_objtype_state[token->obj_type]; @@ -750,9 +714,6 @@ void OS_Lock_Global(OS_object_token_t *token) osal_id_t self_task_id; OS_objtype_state_t *objtype; - /* TODO: void pointer, https://github.com/nasa/osal/issues/765 */ - //OS_CHECK_POINTER(token); - if (token->obj_type < OS_OBJECT_TYPE_USER && token->lock_mode != OS_LOCK_MODE_NONE) { objtype = &OS_objtype_state[token->obj_type]; @@ -784,16 +745,17 @@ void OS_Lock_Global(OS_object_token_t *token) * This makes it different for every operation, and different depending * on what task is calling the function. */ - token->lock_key.key_value = OS_LOCK_KEY_FIXED_VALUE | - ((OS_ObjectIdToInteger(self_task_id) ^ objtype->transaction_count) & 0xFFFFFF); + token->lock_key.key_value = + OS_LOCK_KEY_FIXED_VALUE | ((OS_ObjectIdToInteger(self_task_id) ^ objtype->transaction_count) & 0xFFFFFF); ++objtype->transaction_count; if (objtype->owner_key.key_value != 0) { /* this is almost certainly a bug */ - OS_DEBUG("ERROR: global %u acquired by task 0x%lx when already assigned key 0x%lx\n", (unsigned int)token->obj_type, - OS_ObjectIdToInteger(self_task_id), (unsigned long)objtype->owner_key.key_value); + OS_DEBUG("ERROR: global %u acquired by task 0x%lx when already assigned key 0x%lx\n", + (unsigned int)token->obj_type, OS_ObjectIdToInteger(self_task_id), + (unsigned long)objtype->owner_key.key_value); } else { @@ -802,8 +764,8 @@ void OS_Lock_Global(OS_object_token_t *token) } else { - OS_DEBUG("ERROR: cannot lock global %u for mode %u\n", - (unsigned int)token->obj_type, (unsigned int)token->lock_mode); + OS_DEBUG("ERROR: cannot lock global %u for mode %u\n", (unsigned int)token->obj_type, + (unsigned int)token->lock_mode); } } @@ -816,9 +778,6 @@ void OS_Unlock_Global(OS_object_token_t *token) { OS_objtype_state_t *objtype; - /* TODO: void pointer, https://github.com/nasa/osal/issues/765 */ - //OS_CHECK_POINTER(token); - if (token->obj_type < OS_OBJECT_TYPE_USER && token->lock_mode != OS_LOCK_MODE_NONE) { objtype = &OS_objtype_state[token->obj_type]; @@ -835,8 +794,9 @@ void OS_Unlock_Global(OS_object_token_t *token) objtype->owner_key.key_value != token->lock_key.key_value) { /* this is almost certainly a bug */ - OS_DEBUG("ERROR: global %u released using mismatched key=0x%lx expected=0x%lx\n", (unsigned int)token->obj_type, - (unsigned long)token->lock_key.key_value, (unsigned long)objtype->owner_key.key_value); + OS_DEBUG("ERROR: global %u released using mismatched key=0x%lx expected=0x%lx\n", + (unsigned int)token->obj_type, (unsigned long)token->lock_key.key_value, + (unsigned long)objtype->owner_key.key_value); } objtype->owner_key = OS_LOCK_KEY_INVALID; @@ -846,8 +806,8 @@ void OS_Unlock_Global(OS_object_token_t *token) } else { - OS_DEBUG("ERROR: cannot unlock global %u for mode %u\n", - (unsigned int)token->obj_type, (unsigned int)token->lock_mode); + OS_DEBUG("ERROR: cannot unlock global %u for mode %u\n", (unsigned int)token->obj_type, + (unsigned int)token->lock_mode); } } @@ -867,9 +827,6 @@ void OS_WaitForStateChange(OS_object_token_t *token, uint32 attempts) osal_key_t saved_unlock_key; OS_objtype_state_t *objtype; - /* TODO: void pointer, https://github.com/nasa/osal/issues/765 */ - //OS_CHECK_POINTER(token); - /* * This needs to release the lock, to allow other * tasks to make a change to the table. But to avoid @@ -919,13 +876,6 @@ int32 OS_ObjectIdFinalizeNew(int32 operation_status, OS_object_token_t *token, o { osal_id_t final_id; - /* - * Check parameters - * - * Note "outid" is not checked, because in certain configurations it can be validly null. - */ - OS_CHECK_POINTER(token); - /* if operation was unsuccessful, then clear * the active_id field within the record, so * the record can be re-used later. @@ -970,9 +920,6 @@ int32 OS_ObjectIdFinalizeDelete(int32 operation_status, OS_object_token_t *token { osal_id_t final_id; - /* Check parameters */ - OS_CHECK_POINTER(token); - /* Clear the OSAL ID if successful - this returns the record to the pool */ if (operation_status == OS_SUCCESS) { @@ -1072,9 +1019,6 @@ int32 OS_ObjectIdFindByName(osal_objtype_t idtype, const char *name, osal_id_t * int32 return_code; OS_object_token_t token; - /* Check parameters */ - OS_CHECK_POINTER(object_id); - /* * As this is an internal-only function, calling it will NULL is allowed. * This is required by the file/dir/socket API since these DO allow multiple @@ -1163,14 +1107,6 @@ void OS_ObjectIdTransactionFinish(OS_object_token_t *token, osal_id_t *final_id) { OS_common_record_t *record; - /* TODO: void pointer, https://github.com/nasa/osal/issues/765 */ - /* - * Check parameters - * - * Note "final_id" is not checked, because in certain configurations it can be validly null. - */ - //OS_CHECK_POINTER(token); - if (token->lock_mode == OS_LOCK_MODE_NONE) { /* nothing to do */ @@ -1342,10 +1278,6 @@ int32 OS_ObjectIdAllocateNew(osal_objtype_t idtype, const char *name, OS_object_ ------------------------------------------------------------------*/ void OS_ObjectIdTransferToken(OS_object_token_t *token_from, OS_object_token_t *token_to) { - /* TODO: void pointer, https://github.com/nasa/osal/issues/765 */ - //OS_CHECK_POINTER(token_to); - //OS_CHECK_POINTER(token_from); - /* start with a simple copy */ *token_to = *token_from; @@ -1365,13 +1297,6 @@ void OS_ObjectIdTransferToken(OS_object_token_t *token_from, OS_object_token_t * int32 OS_ObjectIdIteratorInit(OS_ObjectMatchFunc_t matchfunc, void *matcharg, osal_objtype_t objtype, OS_object_iter_t *iter) { - /* - * Check parameters - * - * Note "matcharg" is not checked, because in certain configurations it can be validly null. - */ - OS_CHECK_POINTER(iter); - iter->match = matchfunc; iter->arg = matcharg; iter->limit = OS_GetMaxForObjectType(objtype); @@ -1387,9 +1312,6 @@ int32 OS_ObjectIdIteratorInit(OS_ObjectMatchFunc_t matchfunc, void *matcharg, os ------------------------------------------------------------------*/ bool OS_ObjectFilterActive(void *ref, const OS_object_token_t *token, const OS_common_record_t *obj) { - /* Check parameters */ - OS_CHECK_POINTER(obj); - return OS_ObjectIdDefined(obj->active_id); } @@ -1412,8 +1334,6 @@ bool OS_ObjectIdIteratorGetNext(OS_object_iter_t *iter) { OS_common_record_t *record; bool got_next; - - OS_CHECK_POINTER(iter); got_next = false; iter->token.obj_id = OS_OBJECT_ID_UNDEFINED; @@ -1456,10 +1376,6 @@ int32 OS_ObjectIdIteratorProcessEntry(OS_object_iter_t *iter, int32 (*func)(osal { int32 status; - /* Check parameters */ - OS_CHECK_POINTER(iter); - OS_CHECK_POINTER(func); - /* * This needs to temporarily unlock the global, * call the handler function, then re-lock. @@ -1517,13 +1433,14 @@ void OS_ForEachObject(osal_id_t creator_id, OS_ArgCallback_t callback_ptr, void * See description in API and header file for detail * *-----------------------------------------------------------------*/ -void OS_ForEachObjectOfType(osal_objtype_t idtype, osal_id_t creator_id, OS_ArgCallback_t callback_ptr, void *callback_arg) +void OS_ForEachObjectOfType(osal_objtype_t idtype, osal_id_t creator_id, OS_ArgCallback_t callback_ptr, + void *callback_arg) { OS_object_iter_t iter; OS_creator_filter_t filter; /* TODO: void pointer, https://github.com/nasa/osal/issues/765 */ - //OS_CHECK_POINTER(callback_arg) + // OS_CHECK_POINTER(callback_arg) filter.creator_id = creator_id; filter.user_callback = callback_ptr; diff --git a/src/os/shared/src/osapi-module.c b/src/os/shared/src/osapi-module.c index 96c5d5835..18b6eef26 100644 --- a/src/os/shared/src/osapi-module.c +++ b/src/os/shared/src/osapi-module.c @@ -101,14 +101,6 @@ int32 OS_SymbolLookup_Static(cpuaddr *SymbolAddress, const char *SymbolName, con int32 return_code = OS_ERR_NOT_IMPLEMENTED; OS_static_symbol_record_t *StaticSym = OS_STATIC_SYMTABLE_SOURCE; - /* - * Check parameters - * - * Note "ModuleName" is not checked, because in certain configurations it can be validly null. - */ - OS_CHECK_POINTER(SymbolAddress); - OS_CHECK_POINTER(SymbolName); - while (StaticSym != NULL) { if (StaticSym->Name == NULL) @@ -147,9 +139,6 @@ int32 OS_ModuleLoad_Static(const char *ModuleName) int32 return_code = OS_ERR_NAME_NOT_FOUND; OS_static_symbol_record_t *StaticSym = OS_STATIC_SYMTABLE_SOURCE; - /* Check parameters */ - OS_CHECK_POINTER(ModuleName); - while (StaticSym != NULL) { if (StaticSym->Name == NULL) diff --git a/src/os/shared/src/osapi-printf.c b/src/os/shared/src/osapi-printf.c index 4ed98e732..a6879f986 100644 --- a/src/os/shared/src/osapi-printf.c +++ b/src/os/shared/src/osapi-printf.c @@ -142,12 +142,6 @@ static int32 OS_Console_CopyOut(OS_console_internal_record_t *console, const cha size_t WriteOffset; int32 return_code; - /* Check parameters */ - OS_CHECK_POINTER(console); - OS_CHECK_POINTER(Str); - OS_CHECK_POINTER(NextWritePos); - - return_code = OS_ERROR; pmsg = Str; WriteOffset = *NextWritePos; @@ -263,7 +257,7 @@ void OS_printf(const char *String, ...) int actualsz; /* TODO: void pointer, https://github.com/nasa/osal/issues/765 */ - //OS_CHECK_POINTER(String); + // OS_CHECK_POINTER(String); if (!OS_SharedGlobalVars.Initialized) { diff --git a/src/os/shared/src/osapi-select.c b/src/os/shared/src/osapi-select.c index 3158ebbc3..f74453366 100644 --- a/src/os/shared/src/osapi-select.c +++ b/src/os/shared/src/osapi-select.c @@ -90,10 +90,10 @@ int32 OS_SelectMultiple(OS_FdSet *ReadSet, OS_FdSet *WriteSet, int32 msecs) int32 return_code; /* - * Check parameters - * - * Note "ReadSet" and "WriteSet" are not checked, because in certain configurations they can be validly null. - */ + * Check parameters + * + * Note "ReadSet" and "WriteSet" are not checked, because in certain configurations they can be validly null. + */ /* * This does not currently increment any refcounts. diff --git a/src/os/shared/src/osapi-sockets.c b/src/os/shared/src/osapi-sockets.c index 66f2aeae5..ddfdd44ab 100644 --- a/src/os/shared/src/osapi-sockets.c +++ b/src/os/shared/src/osapi-sockets.c @@ -96,10 +96,6 @@ void OS_CreateSocketName(const OS_object_token_t *token, const OS_SockAddr_t *Ad uint16 port; OS_stream_internal_record_t *sock; - /* TODO: void pointer, https://github.com/nasa/osal/issues/765 */ - //OS_CHECK_POINTER(token); - //OS_CHECK_POINTER(Addr); - sock = OS_OBJECT_TABLE_GET(OS_stream_table, *token); if (OS_SocketAddrToString_Impl(sock->stream_name, OS_MAX_API_NAME, Addr) != OS_SUCCESS) @@ -364,10 +360,10 @@ int32 OS_SocketRecvFrom(osal_id_t sock_id, void *buffer, size_t buflen, OS_SockA int32 return_code; /* - * Check parameters - * - * Note "RemoteAddr" is not checked, because in certain configurations it can be validly null. - */ + * Check parameters + * + * Note "RemoteAddr" is not checked, because in certain configurations it can be validly null. + */ OS_CHECK_POINTER(buffer); OS_CHECK_SIZE(buflen); diff --git a/src/os/shared/src/osapi-task.c b/src/os/shared/src/osapi-task.c index 2cfa88aba..29c60dd4f 100644 --- a/src/os/shared/src/osapi-task.c +++ b/src/os/shared/src/osapi-task.c @@ -85,9 +85,6 @@ static int32 OS_TaskPrepare(osal_id_t task_id, osal_task_entry *entrypt) OS_object_token_t token; OS_task_internal_record_t *task; - /* Check parameters */ - OS_CHECK_POINTER(entrypt); - return_code = OS_ObjectIdGetById(OS_LOCK_MODE_GLOBAL, OS_OBJECT_TYPE_OS_TASK, task_id, &token); if (return_code == OS_SUCCESS) { @@ -182,10 +179,10 @@ int32 OS_TaskCreate(osal_id_t *task_id, const char *task_name, osal_task_entry f OS_task_internal_record_t *task; /* - * Check parameters - * - * Note "stack_pointer" is not checked, because in certain configurations it can be validly null. - */ + * Check parameters + * + * Note "stack_pointer" is not checked, because in certain configurations it can be validly null. + */ OS_CHECK_POINTER(task_id); OS_CHECK_POINTER(function_pointer); OS_CHECK_APINAME(task_name); diff --git a/src/os/shared/src/osapi-time.c b/src/os/shared/src/osapi-time.c index 0400873de..fd834944f 100644 --- a/src/os/shared/src/osapi-time.c +++ b/src/os/shared/src/osapi-time.c @@ -106,12 +106,13 @@ static int32 OS_DoTimerAdd(osal_id_t *timer_id, const char *timer_name, osal_id_ OS_timebase_internal_record_t *timebase; /* - ** Check Parameters + * Check parameters + * + * Note "callback_arg" is not checked, because in certain configurations it can be validly null. */ OS_CHECK_POINTER(timer_id); OS_CHECK_APINAME(timer_name); OS_CHECK_POINTER(callback_ptr); - OS_CHECK_POINTER(callback_arg); /* * Check our context. Not allowed to use the timer API from a timer callback. @@ -219,9 +220,6 @@ static void OS_Timer_NoArgCallback(osal_id_t objid, void *arg) { OS_Timer_ArgWrapper_t Conv; - /* TODO: void pointer, https://github.com/nasa/osal/issues/765 */ - //OS_CHECK_POINTER(arg); - /* * Note - did not write this as simply *((OS_SimpleCallback_t)arg) because * technically you cannot cast a void * to a function pointer. diff --git a/src/os/shared/src/osapi-timebase.c b/src/os/shared/src/osapi-timebase.c index f922b2630..219f3e549 100644 --- a/src/os/shared/src/osapi-timebase.c +++ b/src/os/shared/src/osapi-timebase.c @@ -540,9 +540,6 @@ int32 OS_Milli2Ticks(uint32 milli_seconds, int *ticks) uint64 num_of_ticks; int32 return_code = OS_SUCCESS; - /* Check parameters */ - OS_CHECK_POINTER(ticks); - num_of_ticks = (((uint64)milli_seconds * OS_SharedGlobalVars.TicksPerSecond) + 999) / 1000; /* Check against maximum int32 (limit from some OS's) */ diff --git a/src/unit-test-coverage/shared/src/coveragetest-idmap.c b/src/unit-test-coverage/shared/src/coveragetest-idmap.c index 440d852c8..ee8270dd8 100644 --- a/src/unit-test-coverage/shared/src/coveragetest-idmap.c +++ b/src/unit-test-coverage/shared/src/coveragetest-idmap.c @@ -432,11 +432,6 @@ void Test_OS_ObjectIdFindByName(void) int32 actual = OS_ObjectIdFindByName(OS_OBJECT_TYPE_UNDEFINED, NULL, &objid); UtAssert_True(actual == expected, "OS_ObjectFindIdByName(%s) (%ld) == OS_ERR_NAME_NOT_FOUND", "NULL", (long)actual); - expected = OS_INVALID_POINTER; - actual = OS_ObjectIdFindByName(OS_OBJECT_TYPE_UNDEFINED, TaskName, NULL); - UtAssert_True(actual == expected, "OS_ObjectFindIdByName(%s) (%ld) == OS_INVALID_POINTER", "NULL", (long)actual); - - /* * Pass in a name that is beyond OS_MAX_API_NAME */ From 3b2c729809c970bdc674fe2e7d991ac710d918a8 Mon Sep 17 00:00:00 2001 From: Alex Campbell Date: Wed, 27 Jan 2021 12:44:52 -0500 Subject: [PATCH 3/3] Fix #765, add null pointer check. --- src/os/shared/src/osapi-idmap.c | 3 --- src/os/shared/src/osapi-printf.c | 3 +-- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/src/os/shared/src/osapi-idmap.c b/src/os/shared/src/osapi-idmap.c index d6e16791e..d44cab065 100644 --- a/src/os/shared/src/osapi-idmap.c +++ b/src/os/shared/src/osapi-idmap.c @@ -1439,9 +1439,6 @@ void OS_ForEachObjectOfType(osal_objtype_t idtype, osal_id_t creator_id, OS_ArgC OS_object_iter_t iter; OS_creator_filter_t filter; - /* TODO: void pointer, https://github.com/nasa/osal/issues/765 */ - // OS_CHECK_POINTER(callback_arg) - filter.creator_id = creator_id; filter.user_callback = callback_ptr; filter.user_arg = callback_arg; diff --git a/src/os/shared/src/osapi-printf.c b/src/os/shared/src/osapi-printf.c index a6879f986..5b766193d 100644 --- a/src/os/shared/src/osapi-printf.c +++ b/src/os/shared/src/osapi-printf.c @@ -256,8 +256,7 @@ void OS_printf(const char *String, ...) char msg_buffer[OS_BUFFER_SIZE]; int actualsz; - /* TODO: void pointer, https://github.com/nasa/osal/issues/765 */ - // OS_CHECK_POINTER(String); + BUGCHECK((String) != NULL, ) if (!OS_SharedGlobalVars.Initialized) {