Merge pull request #975 from nasa/integration-candidate

osal Integration candidate: 2021-04-27

.github/workflows/codeql-build.yml

@@ -17,7 +17,6 @@ jobs:
     timeout-minutes: 15
 
     steps:
-      # Checks out a copy of your repository on the ubuntu-latest machine
       - name: Checkout bundle
         uses: actions/checkout@v2
         with:
@@ -38,21 +37,17 @@ jobs:
          languages: c
          queries: +security-extended, security-and-quality
 
-      # Setup the build system
       - name: Set up for build
         run: |
           cp ./cfe/cmake/Makefile.sample Makefile
           cp -r ./cfe/cmake/sample_defs sample_defs
           make prep
 
-      # Build the code
       - name: Build
-        run: |
-          make osal
-          make native/default_cpu1/osal/tests/
-          make native/default_cpu1/osal/unit-test-coverage/
-          make native/default_cpu1/osal/unit-tests/
-          make native/default_cpu1/osal/ut-stubs/
+        run: make -j native/default_cpu1/osal/
+
+      - name: Run tests
+        run: (cd build/native/default_cpu1/osal && make test)
 
       - name: Perform CodeQL Analysis
         uses: github/codeql-action/analyze@v1

.github/workflows/local_unit_test.yml

@@ -0,0 +1,41 @@
+name: "Local Unit Test"
+
+on:
+  push:
+  pull_request:
+
+jobs:
+
+  Local-Unit-Test:
+    runs-on: ubuntu-18.04
+    timeout-minutes: 15
+
+    steps:
+      - name: Install coverage tools
+        run: sudo apt-get install lcov -y
+
+      - name: Checkout submodule
+        uses: actions/checkout@v2
+
+      - name: Set up for build
+        run: |
+          cp Makefile.sample Makefile
+          make ENABLE_UNIT_TESTS=true PERMISSIVE_MODE=true prep
+
+      - name: Build the code
+        run: make -j
+
+      # Baseline lcov and run all tests
+      - name: Test
+        run: make test
+
+      - name: Calculate coverage 
+        run: make lcov | tee lcov_out.txt
+
+      - name: Confirm 100% line coverage
+        run: |
+          if [[ `grep -A 3 "Overall coverage rate" lcov_out.txt | grep lines` != *"100.0%"* ]]; then
+            grep -A 3 "Overall coverage rate" lcov_out.txt
+            echo "Lacks 100.0% line unit test coverage"
+            exit -1
+          fi

.gitignore

@@ -1 +1,2 @@
-build/inc/
+build
+Makefile

Makefile.sample

@@ -0,0 +1,152 @@
+#
+# Operating System Abstraction Layer CMake / GNU make wrapper
+#
+# ABOUT THIS MAKEFILE:
+# It is a GNU-make wrapper that calls the CMake tools appropriately
+# so that setting up a new build is fast and easy with no need to
+# learn the CMake commands.  It also makes it easier to integrate
+# the build with IDE tools such as Eclipse by providing a default
+# makefile that has the common targets such as all/clean/etc.
+#
+# Use of this file is optional.
+#
+# For _ALL_ targets defined in this file the build tree location may
+# be specified via the "O" variable (i.e. make O=<my-build-dir> all).
+# If not specified then the "build" subdirectory will be assumed.
+#
+# This wrapper defines the following major targets:
+#  prep -- Runs CMake to create a new or re-configure an existing build tree
+#    Note that multiple build trees can exist from a single source
+#    Other control options may be passed to CMake via
+#    make variables depending on the mission build scripts.  These will be
+#    cached in the build tree so they do not need to be set again thereafter.
+#
+#  all -- Build all targets in the CMake build tree
+#
+#  install -- Copy all files to the installation tree and run packaging scripts
+#     The "DESTDIR" and "INSTALLPREFIX" environment variables control where the
+#     files are copied
+#
+#  clean -- Clean all targets in the CMake build tree, but not the build tree itself.
+#
+#  distclean -- Entirely remove the build directory specified by "O"
+#      Note that after this the "prep" step must be run again in order to build.
+#      Use caution with this as it does an rm -rf - don't set O to your home dir!
+#
+#  test -- Run all unit tests defined in the build on the host.  This will not
+#      work if the BSPTYPE selection is not compatible with the host.
+#      In that case it is up to the user to copy the executables to the target
+#      and run them.
+#
+#  lcov -- Runs the "lcov" tool on the build tree to collect all code coverage
+#      analysis data and build the reports.  Code coverage data may be created by
+#      the "make test" target above.
+#
+
+# Establish default values for critical variables.  Any of these may be overridden
+# on the command line or via the make environment configuration in an IDE
+O ?= build
+BSPTYPE ?= generic-linux
+BUILDTYPE ?= debug
+INSTALLPREFIX ?= /exe
+DESTDIR ?= $(O)
+
+# The "DESTDIR" variable is a bit more complicated because it should be an absolute
+# path for CMake, but we want to accept either absolute or relative paths.  So if
+# the path does NOT start with "/", prepend it with the current directory.
+ifeq ($(filter /%, $(DESTDIR)),)
+DESTDIR := $(CURDIR)/$(DESTDIR)
+endif
+
+# The "LOCALTGTS" defines the top-level targets that are implemented in this makefile
+# Any other target may also be given, in that case it will simply be passed through.
+LOCALTGTS := prep all clean install distclean test lcov
+OTHERTGTS := $(filter-out $(LOCALTGTS),$(MAKECMDGOALS))
+
+# As this makefile does not build any real files, treat everything as a PHONY target
+# This ensures that the rule gets executed even if a file by that name does exist
+.PHONY: $(LOCALTGTS) $(OTHERTGTS)
+
+# If the target name appears to be a directory (ends in /), do a make all in that directory
+DIRTGTS := $(filter %/,$(OTHERTGTS))
+ifneq ($(DIRTGTS),)
+$(DIRTGTS):
+	$(MAKE) -C $(O)/$(patsubst $(O)/%,%,$(@)) all
+endif
+
+# For any other goal that is not one of the known local targets, pass it to the build
+# as there might be a target by that name.  For example, this is useful for rebuilding
+# single unit test executable files while debugging from the IDE
+FILETGTS := $(filter-out $(DIRTGTS),$(OTHERTGTS))
+ifneq ($(FILETGTS),)
+$(FILETGTS):
+	$(MAKE) -C $(O) $(@)
+endif
+
+# The "prep" step requires extra options that are specified via enviroment variables.
+# Certain special ones should be passed via cache (-D) options to CMake.
+# These are only needed for the "prep" target but they are computed globally anyway.
+#
+# Note this simple makefile just builds for one target, could trivally manage
+# multiple targets by changing build directory.  More complex target
+# list examples are provide by cFE..
+PREP_OPTS := -DOSAL_SYSTEM_BSPTYPE=$(BSPTYPE) -DINSTALL_TARGET_LIST=.
+
+ifneq ($(INSTALLPREFIX),)
+PREP_OPTS += -DCMAKE_INSTALL_PREFIX=$(INSTALLPREFIX)
+endif
+
+ifneq ($(VERBOSE),)
+PREP_OPTS += --trace
+endif
+
+ifneq ($(OMIT_DEPRECATED),)
+PREP_OPTS += -DOSAL_OMIT_DEPRECATED=$(OMIT_DEPRECATED)
+endif
+
+ifneq ($(BUILDTYPE),)
+PREP_OPTS += -DCMAKE_BUILD_TYPE=$(BUILDTYPE)
+endif
+
+ifneq ($(ENABLE_UNIT_TESTS),)
+PREP_OPTS += -DENABLE_UNIT_TESTS=$(ENABLE_UNIT_TESTS)
+endif
+
+ifneq ($(PERMISSIVE_MODE),)
+PREP_OPTS += -DOSAL_CONFIG_DEBUG_PERMISSIVE_MODE=$(PERMISSIVE_MODE)
+endif
+
+all:
+	$(MAKE) --no-print-directory -C "$(O)" all
+
+install:
+	$(MAKE) --no-print-directory -C "$(O)" DESTDIR="${DESTDIR}" install
+
+prep $(O)/.prep:
+	mkdir -p "$(O)"
+	(cd "$(O)" && cmake $(PREP_OPTS) "$(CURDIR)")
+	echo "$(PREP_OPTS)" > "$(O)/.prep"
+
+clean:
+	$(MAKE) --no-print-directory -C "$(O)" clean
+
+distclean:
+	rm -rf "$(O)"
+
+# Grab lcov baseline before running tests
+test:
+	lcov --capture --initial --directory $(O) --output-file $(O)/coverage_base.info
+	$(MAKE) --no-print-directory -C "$(O)" test
+
+lcov:
+	lcov --capture --rc lcov_branch_coverage=1 --directory $(O) --output-file $(O)/coverage_test.info
+	lcov --rc lcov_branch_coverage=1 --add-tracefile $(O)/coverage_base.info --add-tracefile $(O)/coverage_test.info --output-file $(O)/coverage_total.info
+	genhtml $(O)/coverage_total.info --branch-coverage --output-directory $(O)/lcov
+	@/bin/echo -e "\n\nCoverage Report Link: file:$(CURDIR)/$(O)/lcov/index.html\n"
+
+
+# Make all the commands that use the build tree depend on a flag file
+# that is used to indicate the prep step has been done.  This way
+# the prep step does not need to be done explicitly by the user
+# as long as the default options are sufficient.
+$(filter-out prep distclean,$(LOCALTGTS)): $(O)/.prep

README.md

@@ -11,6 +11,21 @@ The autogenerated OSAL user's guide can be viewed at <https://github.com/nasa/cF
 
 ## Version History
 
+### Development Build: v5.1.0-rc1+dev411
+
+- [docs] Clarifies that that zero will be returned on EOF condition in the API documentation for OS_read/write/TimedRead/TimedWrite. In the case of the timed API calls, the `OS_ERR_TIMEOUT` status code will be returned if the timeout expired without the handle becoming readable/writable during that time.
+- Addresses a shortcomings in the UT Assert hook functions. Namely the assumed return type of int32 which is not always the case.
+- Adds the concept of a "handler" function to UT assert to replace hard-coded custom logic in UT assert. A handler is the custom logic that exists between the hook function and the return to the stub caller. The handler is directly responsible for setting all outputs.
+- Adds a script to auto-generate stub functions that match this pattern. Given an API header file, the script extracts
+the declarations, and generates a source file with stub definitions that rely on a separate handler to deal with the needed outputs.
+- Refactors `os-shared-printf.h`) into two parts to improve the compatibility with the script method.
+- Updates all existing stubs in OSAL to use the auto-generated stub logic from the script, created directly from the C header. This ensures that stubs will match the FSW implementation.
+- [continuous-integration] Adds a local osal-specific makefile to help build unit tests. Adds a new github workflow that runs the unit tests in both the context of the bundle configuration and the local OSAL config. Verifies 100% line coverage.
+- Fixes incorrect token use in `OS_SocketAccept`. Enables the `network-api-test` to handle multiple connections that re-use the same acceptor socket between them.
+- Promotes the `OS_CONFIG_CONSOLE_ASYNC` option into the shared layer to remove duplicate implementation code and add more coverage testing.
+- Adds an osconfig option to allow the user to elect this mode at configuration time.
+
+
 ### Development Build: v5.1.0-rc1+dev393
 
 - Changes parameter names to avoid collisions. Renames `access` as `access_mode` in `osapi-file.h`. Renames `time` as `TimeSp` in `os-impl-posix-gettime.c`.

default_config.cmake

@@ -180,6 +180,31 @@ set(OSAL_CONFIG_DEBUG_PRINTF                    FALSE
     CACHE BOOL "Controls inclusion of OS_DEBUG statements in the code"
 )
 
+#
+# OS_CONFIG_CONSOLE_ASYNC
+# ----------------------------------
+#
+# Controls whether the console device writes (OS_printf) will be deferred
+# to a separate utility task or handled directly by the calling task.
+#
+# If set FALSE, the utility task WILL NOT be spawned, and all OS_printf()
+# calls will be synchronously written to the console device.
+#
+# If set TRUE, an extra utility task WILL be spawned, and the data from
+# all OS_printf() calls will be written to an output queue which is then
+# transferred to the console device by the utility task.
+#
+# When this is TRUE (default), it may improve real time performance by not
+# requiring the caller to delay on a potentially slow console device output.
+#
+# However decoupling in this manner requires creation of an extra task and
+# stack to handle the output, and a side effect is that the OS_printf() output
+# can become decoupled from the event/task where it actually occurred, or
+# messages might appear in a different order than they originally occurred.
+#
+set(OSAL_CONFIG_CONSOLE_ASYNC                   TRUE
+    CACHE BOOL "Controls spawning of a separate utility task for OS_printf"
+)
 
 #############################################
 # Resource Limits for the OS API

osconfig.h.in

@@ -44,6 +44,7 @@
 #cmakedefine OSAL_CONFIG_INCLUDE_SHELL
 #cmakedefine OSAL_CONFIG_DEBUG_PRINTF
 #cmakedefine OSAL_CONFIG_DEBUG_PERMISSIVE_MODE
+#cmakedefine OSAL_CONFIG_CONSOLE_ASYNC
 
 #cmakedefine OSAL_CONFIG_BUGCHECK_DISABLE
 #cmakedefine OSAL_CONFIG_BUGCHECK_STRICT

src/os/inc/osapi-file.h

@@ -160,6 +160,9 @@ int32 OS_close(osal_id_t filedes);
  *
  * Reads up to nbytes from a file, and puts them into buffer.
  *
+ * If the file position is at the end of file (or beyond, if the OS allows) then this
+ * function will return 0.
+ *
  * @param[in]  filedes  The handle ID to operate on
  * @param[out] buffer   Storage location for file data
  * @param[in]  nbytes   Maximum number of bytes to read
@@ -171,6 +174,7 @@ int32 OS_close(osal_id_t filedes);
  * @retval #OS_INVALID_POINTER if buffer is a null pointer
  * @retval #OS_ERROR if OS call failed
  * @retval #OS_ERR_INVALID_ID if the file descriptor passed in is invalid
+ * @retval 0 if at end of file/stream data
  */
 int32 OS_read(osal_id_t filedes, void *buffer, size_t nbytes);
 
@@ -192,6 +196,7 @@ int32 OS_read(osal_id_t filedes, void *buffer, size_t nbytes);
  * @retval #OS_INVALID_POINTER if buffer is NULL
  * @retval #OS_ERROR if OS call failed
  * @retval #OS_ERR_INVALID_ID if the file descriptor passed in is invalid
+ * @retval 0 if file/stream cannot accept any more data
  */
 int32 OS_write(osal_id_t filedes, const void *buffer, size_t nbytes);
 
@@ -201,28 +206,37 @@ int32 OS_write(osal_id_t filedes, const void *buffer, size_t nbytes);
  *
  * This implements a time-limited read and is primarily intended for use with
  * sockets but may also work with any other stream-like resource that the underlying
- * OS supports.
+ * OS supports, such as pipes or special devices.
  *
  * If data is immediately available on the file/socket, this will return that data
  * along with the actual number of bytes that were immediately available.  It will
  * not block.
  *
- * If no data is immediately available, this will wait up to the given timeout for
+ * If the file position is at the end of file or end of stream data (e.g. if the remote
+ * end has closed the connection), then this function will immediately return 0 without
+ * blocking for the timeout period.
+ *
+ * If no data is immediately available, but the underlying resource/stream is still
+ * connected to a peer, this will wait up to the given timeout for additional
  * data to appear.  If no data appears within the timeout period, then this returns
- * an error code (not zero).
+ * the #OS_ERROR_TIMEOUT status code.  This allows the caller to differentiate
+ * an open (but idle) socket connection from a connection which has been closed
+ * by the remote peer.
  *
  * In all cases this will return successfully as soon as at least 1 byte of actual
  * data is available.  It will not attempt to read the entire input buffer.
  *
  * If an EOF condition occurs prior to timeout, this function returns zero.
  *
- * @param[in] filedes   The handle ID to operate on
- * @param[in] buffer    Source location for file data
- * @param[in] nbytes    Maximum number of bytes to read
- * @param[in] timeout   Maximum time to wait, in milliseconds (OS_PEND = forever)
+ * @param[in]  filedes   The handle ID to operate on
+ * @param[out] buffer    Storage location for file data
+ * @param[in]  nbytes    Maximum number of bytes to read
+ * @param[in]  timeout   Maximum time to wait, in milliseconds (OS_PEND = forever)
  *
- * @return Byte count on success, zero for timeout, or appropriate error code,
- *         see @ref OSReturnCodes
+ * @returns Byte count on success or appropriate error code, see @ref OSReturnCodes
+ * @retval #OS_ERROR_TIMEOUT if no data became available during timeout period
+ * @retval #OS_ERR_INVALID_ID if the file descriptor passed in is invalid
+ * @retval 0 if at end of file/stream data
  */
 int32 OS_TimedRead(osal_id_t filedes, void *buffer, size_t nbytes, int32 timeout);
 
@@ -252,8 +266,10 @@ int32 OS_TimedRead(osal_id_t filedes, void *buffer, size_t nbytes, int32 timeout
  * @param[in] nbytes    Maximum number of bytes to read
  * @param[in] timeout   Maximum time to wait, in milliseconds (OS_PEND = forever)
  *
- * @return Byte count on success, zero for timeout, or appropriate error code,
- *         see @ref OSReturnCodes
+ * @return A non-negative byte count or appropriate error code, see @ref OSReturnCodes
+ * @retval #OS_ERROR_TIMEOUT if no data became available during timeout period
+ * @retval #OS_ERR_INVALID_ID if the file descriptor passed in is invalid
+ * @retval 0 if file/stream cannot accept any more data
  */
 int32 OS_TimedWrite(osal_id_t filedes, const void *buffer, size_t nbytes, int32 timeout);
 

src/os/inc/osapi-version.h

@@ -34,7 +34,7 @@
 /*
  * Development Build Macro Definitions
  */
-#define OS_BUILD_NUMBER   393
+#define OS_BUILD_NUMBER   411
 #define OS_BUILD_BASELINE "v5.1.0-rc1"
 
 /*

src/os/posix/inc/os-impl-console.h

@@ -36,7 +36,6 @@
 /* Console device */
 typedef struct
 {
-    bool  is_async;
     sem_t data_sem;
 } OS_impl_console_internal_record_t;