Skip to content

Commit

Permalink
Fix nasa#688, Initial macro implementation and config options
Browse files Browse the repository at this point in the history
Add macros for configurable behavior of argument bug checking.

- BUGCHECK for checking argument values which should never happen
   and indicate bugs if they do.
- ARGCHECK for checking argument values which may happen and can
   be mitigated if they do.

The behavior of BUGCHECK is influenced by two new OSAL config
options, which can disable it completely or make it strict/enforcing
such that it will abort() for debugging if a condition is not met.
  • Loading branch information
jphickey committed Dec 15, 2020
1 parent 8dd7415 commit f9e9b28
Show file tree
Hide file tree
Showing 6 changed files with 266 additions and 26 deletions.
47 changes: 47 additions & 0 deletions default_config.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,53 @@
#
##########################################################################

##############################################################
# Argument/Bug-checking options
##############################################################

# OSAL_CONFIG_BUGCHECK_DISABLE
# ----------------------------------
#
# Disable/compile-out the "bugcheck" macro
#
# The bugcheck macro is used to validate the inputs to functions and/or
# assert on other conditions that should _always_ be true. If any of these
# conditions ever evaluate as false, it indicates a bug in the code -
# either in the OSAL or the application which invoked OSAL.
#
# If set FALSE (default), then the OSAL bugcheck macro will evaluate its
# boolean conditional and generate an action if that conditional evaulates
# false. (The specific action to take is configured via a different
# directive -- see OSAL_CONFIG_BUGCHECK_STRICT).
#
# These extra bug checks do consume a slight bit of code+data space as
# well as some runtime CPU cycles on every call, depending on the conditions
# being tested.
#
# Once the application has reached a sufficient level of stability and
# confidence is obtained that these bug checks are not possible to be
# triggered, this directive may be set TRUE which disables the bug checks
# completely - rendering these statements as no-ops.
#
set(OSAL_CONFIG_BUGCHECK_DISABLE FALSE)


# OSAL_CONFIG_BUGCHECK_STRICT
# ----------------------------------
#
# Select a strict implementation for the "bugcheck" macro
#
# If set FALSE (default), then the OSAL bugcheck macro will generate a
# debug message and return an error code if the conditional evaluates
# as false. This is a soft error - the application will get the
# error code and keep running.
#
# If set to TRUE, then any failure of any bugcheck macro is considered
# fatal and will trigger an abort(). On many platforms this will
# generate an abnormal application exit with a core file for debugging.
#
set(OSAL_CONFIG_BUGCHECK_STRICT FALSE)


##############################################################
# Code/Feature Selection Options for the OSAL implementation
Expand Down
49 changes: 26 additions & 23 deletions osconfig.h.in
Original file line number Diff line number Diff line change
Expand Up @@ -20,38 +20,41 @@

/**
* \brief Configuration file Operating System Abstraction Layer
*
*
* The specific definitions in this file may only be modified
* by setting the respective OSAL configuration options in the CMake
* build.
* by setting the respective OSAL configuration options in the CMake
* build.
*
* Any direct modifications to the generated copy will
* be overwritten each time CMake executes.
* be overwritten each time CMake executes.
*
* \note This file was automatically generated by CMake from
* @CMAKE_CURRENT_SOURCE_DIR@/default_config.cmake
*/

#ifndef OSCONFIG_H
#define OSCONFIG_H

/*
* OSAL feature selection options from build config
/*
* OSAL feature selection options from build config
*/
#cmakedefine OSAL_CONFIG_INCLUDE_DYNAMIC_LOADER
#cmakedefine OSAL_CONFIG_INCLUDE_NETWORK
#cmakedefine OSAL_CONFIG_INCLUDE_STATIC_LOADER
#cmakedefine OSAL_CONFIG_INCLUDE_NETWORK
#cmakedefine OSAL_CONFIG_INCLUDE_STATIC_LOADER
#cmakedefine OSAL_CONFIG_INCLUDE_SHELL
#cmakedefine OSAL_CONFIG_DEBUG_PRINTF
#cmakedefine OSAL_CONFIG_DEBUG_PERMISSIVE_MODE
#cmakedefine OSAL_CONFIG_DEBUG_PRINTF
#cmakedefine OSAL_CONFIG_DEBUG_PERMISSIVE_MODE

#cmakedefine OSAL_CONFIG_BUGCHECK_DISABLE
#cmakedefine OSAL_CONFIG_BUGCHECK_STRICT

/*
/*
* OSAL resource limits from build config
*
* (These are prefixed with OS_ for compatibility
* with existing code referencing these symbols)
*/

/**
* \brief The maximum number of to support
*
Expand Down Expand Up @@ -140,8 +143,8 @@

/**
* \brief The maximum length of OSAL file names
*
* This limit applies specifically to the file name portion, not the
*
* This limit applies specifically to the file name portion, not the
* directory portion, of a path name.
*
* Based on the OSAL_CONFIG_MAX_FILE_NAME configuration option
Expand Down Expand Up @@ -175,14 +178,14 @@
* \brief The maximum size of the socket address structure
*
* This is part of the Socket API, and should be set large enough to hold
* the largest address type in use on the target system.
* the largest address type in use on the target system.
*
* Based on the OSAL_CONFIG_SOCKADDR_MAX_LEN configuration option
*/
#define OS_SOCKADDR_MAX_LEN @OSAL_CONFIG_SOCKADDR_MAX_LEN@

/**
* \brief The maximum size of output produced by a single OS_printf()
* \brief The maximum size of output produced by a single OS_printf()
*
* Based on the OSAL_CONFIG_PRINTF_BUFFER_SIZE configuration option
*/
Expand Down Expand Up @@ -234,7 +237,7 @@
#define OS_QUEUE_MAX_DEPTH @OSAL_CONFIG_QUEUE_MAX_DEPTH@

/**
* \brief The name of the temporary file used to store shell commands
* \brief The name of the temporary file used to store shell commands
*
* This configuration is only applicable if shell support is enabled, and
* only necessary/relevant on some OS implementations.
Expand All @@ -251,14 +254,14 @@
*
* Based on the OSAL_CONFIG_PRINTF_CONSOLE_NAME configuration option
*/
#define OS_PRINTF_CONSOLE_NAME "@OSAL_CONFIG_PRINTF_CONSOLE_NAME@"
#define OS_PRINTF_CONSOLE_NAME "@OSAL_CONFIG_PRINTF_CONSOLE_NAME@"

/*
/*
* OSAL fixed resource limits
*
* The resource limits here are not user-configurable, but
* may be changed in a future revision of OSAL, so it is
* still present in osconfig.h along with the others.
* may be changed in a future revision of OSAL, so it is
* still present in osconfig.h along with the others.
*/

/**
Expand All @@ -269,7 +272,7 @@
#define OS_MAX_CONSOLES 1

/**
* \brief The system-specific file extension used on loadable module files
* \brief The system-specific file extension used on loadable module files
*
* Fixed value based on system selection, not user configurable.
*/
Expand Down
5 changes: 3 additions & 2 deletions src/os/inc/osapi-error.h
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,9 @@ typedef char os_err_name_t[OS_ERROR_NAME_LENGTH];
#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) \
*/
#define OS_ERR_OPERATION_NOT_SUPPORTED (-38) /**< @brief Requested operation is not support on the supplied object(s) */
#define OS_ERR_INVALID_SIZE (-40) /**< @brief Invalid Size */

/*
** Defines for File System Calls
*/
Expand Down
139 changes: 139 additions & 0 deletions src/os/inc/osapi-macros.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,139 @@
/*
* NASA Docket No. GSC-18,370-1, and identified as "Operating System Abstraction Layer"
*
* Copyright (c) 2019 United States Government as represented by
* the Administrator of the National Aeronautics and Space Administration.
* All Rights Reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

/*
* @file osapi-macros.h
*/

#ifndef OSAPI_MACROS_H
#define OSAPI_MACROS_H

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

#include "osconfig.h"
#include "common_types.h"

#ifdef OSAL_CONFIG_BUGCHECK_DISABLE

/**
* @brief Placeholder for BUGCHECK
*
* When OSAL_CONFIG_BUGCHECK_DISABLE is specified, then
* the BUGCHECK/BUGREPORT macros become no-ops.
*/
#define BUGCHECK(...)

/**
* @brief Placeholder for BUGREPORT
*
* When OSAL_CONFIG_BUGCHECK_DISABLE is specified, then
* the BUGCHECK/BUGREPORT macros become no-ops.
*/
#define BUGREPORT(...)

#else /* Bug checking enabled */

#ifdef OSAL_CONFIG_BUGCHECK_STRICT

/*
* This BUGREPORT implementation aborts the application so that the applicaiton
* can be debugged immediately. This prints the message direct to stderr, which is
* typically not buffered, so it should appear on the console before the abort occurs,
* but may appear out of order with respect to calls to OS_printf().
*/
#define BUGREPORT(...) \
do \
{ \
fprintf(stderr, __VA_ARGS__); \
abort(); \
} while (false)

#else

#include "osapi-printf.h"

/*
* This BUGREPORT simply prints the message using OS_printf, which is buffered. This
* has a minimal realtime impact as it only copies to the buffer, and the print will
* appear in order with respect to other calls to OS_printf() by the application.
*/
#define BUGREPORT(...) OS_printf(__VA_ARGS__)

#endif /* OSAL_CONFIG_BUGCHECK_STRICT */

/**
* @brief Basic Bug-Checking macro
*
* This macro checks a conditional, and if it is FALSE, then it generates a report -
* which may in turn contain additional actions.
*
* BUGCHECK should only be used for conditions which are critical and must always be true.
* If such a condition is ever false then it indicates a bug in the application which
* must be resolved. It may or may not be possible to continue operation if a bugcheck
* fails.
*
* @sa ARGCHECK for checking non-critical values
*/
#define BUGCHECK(cond, errcode) \
if (!(cond)) \
{ \
BUGREPORT("\n**BUG** %s():%d:check \'%s\' FAILED --> %s\n\n", __func__, __LINE__, #cond, #errcode); \
return errcode; \
}

#endif /* OSAL_CONFIG_BUGCHECK_DISABLE */

/**
* @brief Generic argument checking macro for non-critical values
*
* This macro checks a conditional that is expected to be true, and return a value
* if it evaluates false.
*
* ARGCHECK can be used to check for out of range or other invalid argument conditions
* which may (validly) occur at runtime and do not necessarily indicate bugs in the
* application.
*
* These argument checks are NOT considered a fatal errors. The application
* continues to run normally. This does not report the error on the console.
*
* As such, ARGCHECK actions are always compiled in - not selectable at compile-time.
*
* @sa BUGCHECK for checking critical values that indicate bugs
*/
#define ARGCHECK(cond, errcode) \
if (!(cond)) \
{ \
return errcode; \
}

/**
* @brief String length limit check macro
*
* This macro is a specialized version of ARGCHECK that confirms a string will fit
* into a buffer of the specified length, and return an error code if it will not.
*
* @note this uses ARGCHECK, thus treating a string too long as a normal runtime
* (i.e. non-bug) error condition with a typical error return to the caller.
*/
#define LENGTHCHECK(str, len, errcode) ARGCHECK(memchr(str, '\0', len), errcode)

#endif
1 change: 1 addition & 0 deletions src/os/inc/osapi.h
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@
#include "osapi-file.h"
#include "osapi-filesys.h"
#include "osapi-heap.h"
#include "osapi-macros.h"
#include "osapi-idmap.h"
#include "osapi-module.h"
#include "osapi-mutex.h"
Expand Down
51 changes: 50 additions & 1 deletion src/os/shared/inc/os-shared-globaldefs.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
#include "common_types.h"
#include "osapi-constants.h"
#include "osapi-error.h"
#include "osapi-macros.h"

/*
* The "common_record" is part of the generic ID mapping -
Expand Down Expand Up @@ -89,4 +90,52 @@ extern void OS_DebugPrintf(uint32 Level, const char *Func, uint32 Line, const ch
#define OS_DEBUG(...)
#endif

#endif /* OS_SHARED_GLOBALDEFS_H */
/*
* An OSAL-specific check macro for NULL pointer.
* Checked via BUGCHECK - considered a bug/fatal error if check fails.
*
* Returns OS_INVALID_POINTER if pointer is NULL.
*/
#define OS_CHECK_POINTER(ptr) BUGCHECK((ptr) != NULL, OS_INVALID_POINTER)

/*
* An OSAL-specific check macro for an input buffer size.
* Checked via ARGCHECK - non-fatal if check fails.
*
* Returns OS_ERR_INVALID_SIZE if size is 0.
*
* Also returns OS_ERR_INVALID_SIZE if size is excessively large.
* Currently (UINT32_MAX/2) is used as the upper limit, as some API calls
* (e.g. read/write) return a size as an int32 type, and therefore the
* operation cannot exceed the bounds of this type.
*/
#define OS_CHECK_SIZE(val) ARGCHECK((val) > 0 && (val) < (UINT32_MAX/2), OS_ERR_INVALID_SIZE)

/*
* An OSAL-specific check macro for arbitrary string argument validation.
*
* First confirms string is not null using OS_CHECK_POINTER, then checks the maximum
* length of the string using LENGTHCHECK.
*/
#define OS_CHECK_STRING(str, maxlen, errcode) \
do \
{ \
OS_CHECK_POINTER(str); \
LENGTHCHECK(str, maxlen, errcode); \
} while (0)

/*
* An OSAL-specific check macro for object name strings.
*
* Returns OS_ERR_NAME_TOO_LONG if length is exceeded.
*/
#define OS_CHECK_APINAME(str) OS_CHECK_STRING(str, OS_MAX_API_NAME, OS_ERR_NAME_TOO_LONG)

/*
* An OSAL specific argument check macro for path names
*
* Returns OS_FS_ERR_PATH_TOO_LONG if length is exceeded.
*/
#define OS_CHECK_PATHNAME(str) OS_CHECK_STRING(str, OS_MAX_PATH_LEN, OS_FS_ERR_PATH_TOO_LONG)

#endif /* OS_SHARED_GLOBALDEFS_H */

0 comments on commit f9e9b28

Please sign in to comment.