search

nasa / osal

The Core Flight System (cFS) Operating System Abstraction Layer (OSAL)
Apache License 2.0
555 stars 217 forks source link

update OSAL for Class A & Associated unit tests #45

Closed skliper closed 5 years ago

skliper commented 5 years ago

The currently released OSAL unit tests may not fully run with OSAL 4.1.1.

JSC has made updates so that they run with Linux and ARINC653 OSAL 4.1.1.

JSC is currently updating to work with VxWorks 6.7.

These updates are being tracked in the JSC subversion repo and need to be pushed into a proper git branch and further work continued from there.

skliper commented 5 years ago

Imported from trac issue 22. Created by sduran on 2015-02-26T14:12:23, last modified: 2016-03-07T15:56:36

skliper commented 5 years ago

Trac comment by jphickey on 2015-02-26 14:26:37:

FYI - This is exactly the type of thing targeted by my trac #40.

The idea here is that all tests should run using ANY supported OSAL BSP, not just those that have been explicitly added to the unit test code. This will make sure that the unit tests remain future proof as new BSPs are added.

skliper commented 5 years ago

Trac comment by glimes on 2015-02-26 15:33:40:

Current status for 32-bit X86 Linux unit tests as being run by my tame Bamboo server today on the ic-trac-18-2015-0212 branch (currently at [changeset:f22ae86]) are:

{{{ FAILED [ ] OS_GetErrorName - #26 Undefined Error FAILED [ ] OS_mv - #29 Nominal FAILED [ ] OS_fsBytesFree - #27 Nominal FAILED [ ] OS_mkfs - #28 Nominal FAILED [ ] OS_initfs - #28 Nominal FAILED [ ] OS_fsBlocksFree - #27 Nominal }}}

skliper commented 5 years ago

Trac comment by jphickey on 2015-03-19 09:50:10:

The filesystem failures are (mostly) due to the BSP not creating the necessary mount points, and the Error Name failure was due to checking for a too-specific string that is just different in this version of OSAL.

These were all minor fixes - I rolled these into my current branch for trac #40 since this updated the pc-linux BSP for other reasons. With this change the build tests all pass and it shows green.

skliper commented 5 years ago

Trac comment by tngo on 2015-03-26 14:26:39:

The git osal branch created for this ticket is "trac-22-update-osal-unit-tests".

skliper commented 5 years ago

Trac comment by glimes on 2015-04-10 12:49:32:

Starting to merge your changes in now.

We may need to talk at some point about line-termination characters.

skliper commented 5 years ago

Trac comment by lprokop on 2015-06-04 15:51:20:

Expanding Scope of this ticket beyond unit tests to include vxworks OSAL source code updates to all OSAL files necessary to pursue a Class A safety-critical classification of this software for use by Orion and potentially others. The CFS AES project performed static code analysis utilizing the Understand tool for all MISRA rules, ran static cppcheck tool for potential safety concerns, ran the existing unit tests on a vxworks/LEON3 target with !WindRiver Coverage analysis enabled to determine unit test code coverage, and held a general code inspection process. Specific changes as a result will be as follows:

Subsequent commits should address these issues.

skliper commented 5 years ago

Trac comment by jphickey on 2015-06-04 16:19:44:

While these sound like great changes, I only request that these be pushed as separate change sets if possible, rather than one big lump-sum. Can we possibly create a different trac ticket to merge in the MISRA/cppcheck/compiler warning fixes vs. the UT modifications?

Having smaller change sets just makes it easier to review and (usually) easier to merge them forward up to the latest development branch.

skliper commented 5 years ago

Trac comment by sduran on 2015-06-10 10:57:34:

We are trying to check in the changes in small pieces. Since the osal code and the matching unit tests have to be built with each other, we are keeping them in the same branch.

skliper commented 5 years ago

Trac comment by lprokop on 2015-06-10 17:49:40:

commit:  [changeset:5d51482] osfile, osfilesys, ostimer changes for selected MISRA/code inspection findings

commit:  [changeset:c05b231] iosapi.c changes for selected MISRA/code inspection findings

commit:  [changeset:6bf0b29] osnetwork.c/osloader.c changes for MISRA/code inspection findings

skliper commented 5 years ago

Trac comment by glimes on 2015-06-11 15:29:34:

I am a bit concerned about the "osprintf.c" sources, which appear to be LGPL licensed (making our use of this source a license violation).

If we can resolve the license issues, we would also want to fix up the code to use va_arg properly as using pointer manipulations in place of va_arg makes some presumptions about the varargs implementation that are unsupportable.

skliper commented 5 years ago

Trac comment by jphickey on 2015-06-11 17:31:06:

My comments:

  1. I agree fully with Greg's comment above regarding osprintf.c sources. This appears to be an LGPL library and we cannot simply grab it and use it if that is the case.

  2. I have started to review the changes in e.g. [changeset:5d51482] and I'm concerned that this appears to just be blindly changing {{{int}}} to {{{int32}}} any time the MISRA inspector complained about it. This is not correct!

Just because MISRA points something out doesn't mean it's wrong. {{{int}}} (not int32) is actually the correct C type to use in most of these cases //because the code is interacting with the C library//.

If a C library call defines the return type as an {{{int}}} we should store it in an {{{int}}}. This is especially true if the library also defines constants to compare it against (e.g. the {{{OK}}} and {{{ERROR}}} constants). It is entirely possible that a comparison such as:

{{{ if (result == ERROR) }}}

Can be "true" if both {{{result}}} and {{{ERROR}}} are the same type (e.g. int), but "false" if they are different fundamental types (e.g. int32 and int).

Please see my CFE ticket for a detailed explanation of how this can happen: https://babelfish.arc.nasa.gov/trac/cfs_cfe/ticket/26

(Note I didn't fix this for theoretical reasons - it actually doesn't equate on a 64-bit processor if you compare a "long" constant to a negative "int32" value, and this actually caused code to fail on a 64-bit build)

skliper commented 5 years ago

Trac comment by lprokop on 2015-06-17 17:31:48:

[changeset:7376f89] Added

skliper commented 5 years ago

Trac comment by lprokop on 2015-06-18 11:34:14:

Commit: [changeset:05a6a3f] More MISRA

skliper commented 5 years ago

Trac comment by glimes on 2015-06-18 11:43:08:

Is there a document handy that could be included in the project to indicate exactly what rules are being enforced?

skliper commented 5 years ago

Trac comment by lprokop on 2015-06-18 15:38:10:

Replying to [comment:17 glimes]:

Is there a document handy that could be included in the project to indicate exactly what rules are being enforced? Not yet, but there will be. I'm capturing it in a test plan, procedures & report document.

skliper commented 5 years ago

Trac comment by lprokop on 2015-06-25 17:05:14:

Replying to [comment:14 jphickey]:

Agreed on part 2 of this comment, Joe. We were going back and forth on this one, change all so that you can clearly see everywhere that an "int" has to change if the common_types.h is changed based on migrating processors, or selectively in your case, not doing it for library or vxworks OS calls that expect or return the base type "int". Since they were ultimately the same base type, thought no harm, but good example. So I went through all changes, and reverted those utilized in library/os calls or compared to return values for those functions. Will commit this after testing... thanks!

skliper commented 5 years ago

Trac comment by lprokop on 2015-06-26 17:20:04:

commit: [changeset:ec67295] Updated osapi.c for some race conditions, other files changed to revert blanket "int" to "int32" change retaining "int" with vxworks/lib functions.

skliper commented 5 years ago

Trac comment by abrown4 on 2015-07-01 17:42:03:

commit: [changeset:2c8edbe] Added ut-assert to unit-test-coverage For coverage testing. This can be moved or replaced later.

commit: [changeset:5cc4968] Added coverage tests for osnetwork.c
Identified issue #84 when testing with "make NONET=TRUE" (see Makefile). 100% coverage.

commit: [changeset:b412254] Add coverage tests for ostimer.c (with errors).
Identified issues #85, #88, #89, #92. Not all tests pass in this changeset. Maximum coverage achieved. (One statement, ostimer.c line 262, can't be covered due to the logic immediately preceeding it.)

skliper commented 5 years ago

Trac comment by abrown4 on 2015-07-06 19:09:37:

commit: [changeset:5b3e2b3] "Updated ostimer.c (to 100%) coverage with static fcn."
ostimer.c is now 100% covered, however tests fail given the issues that were discovered (see above).

skliper commented 5 years ago

Trac comment by abrown4 on 2015-07-07 18:00:53:

commit: [changeset:124dd01] Fixed incorrect logic in VxWorks OS_TimespecToUsec(), trac #85. Built upon ostimer.c coverage tests on Track #45.

skliper commented 5 years ago

Trac comment by abrown4 on 2015-07-07 18:06:26:

commit: [changeset:baf36c4] Fixed incorrect logic in POSIX OS_TimespecToUsec(), Trac #85. Built upon ostimer.c coverage tests on Track #45.

skliper commented 5 years ago

Trac comment by abrown4 on 2015-07-07 18:38:03:

commit: [changeset:cb5032b], Fixed "Unfreed" Table Entries after failure, VxWorks OS_TimerCreate(). Trac #89. Built upon ostimer.c coverage tests on Track #45.

skliper commented 5 years ago

Trac comment by abrown4 on 2015-07-08 13:13:17:

commit: [changeset:1a6e450], Fixed VxWorks ostimer.c to properly use semaphore, Trac #92. Built upon ostimer.c coverage tests on Track #45. Made unit test changes to differentiate between failures due to Trac #92 and Trac #88.

skliper commented 5 years ago

Trac comment by abrown4 on 2015-07-08 16:50:40:

commit: [changeset:37dc473] Fixed ostimer.c to prevent unterminated strings, Track #88. Built upon ostimer.c coverage tests on Track #45. Bumped unit tests to 100% coverage with logic change.

skliper commented 5 years ago

Trac comment by abrown4 on 2015-07-09 14:10:21:

commit: [changeset:9fccc17] Switching default osnetwork.c coverage build to 32-bit.

skliper commented 5 years ago

Trac comment by abrown4 on 2015-07-13 19:23:17:

commit: [changeset:1e86ff7] Trac #45, Coverage tests for VxWorks osloader.c (in linux). Fixed header typos in osnetwork_testcase.c.

skliper commented 5 years ago

Trac comment by dasp on 2015-07-14 11:19:42:

commit: [changeset:9519e1c7] Added UtAssert unit tests for osapi.c and osfileapi.c for VxWorks

skliper commented 5 years ago

Trac comment by abrown4 on 2015-07-15 11:57:17:

Identified issues #98, #99, #100, #101 for vxworks osloader.c.

skliper commented 5 years ago

Trac comment by dasp on 2015-07-17 13:38:13:

commit:[changeset:730b2d2] Update to UtAssert unit test adaptor header file for osapi to fix compilation issue

skliper commented 5 years ago

Trac comment by abrown4 on 2015-07-17 14:33:01:

Identified issues #103, #104, & #105 for osfileapi.c. Identified issues #106, & #107 for osfileapi.c.

skliper commented 5 years ago

Trac comment by abrown4 on 2015-07-17 18:03:09:

commit: [changeset:82f215e] Fixed osnetwork return codes with no network. Fixes #84. Tested with coverage tests on Trac #45 branch and in CFS_TST 0d3a682.

skliper commented 5 years ago

Trac comment by abrown4 on 2015-07-20 16:14:49:

commit [changeset:ecfe66e] Trac #99, osloader mutex unit test update. commit [changeset:be8d33e] Trac #99, osloader mutex fix.

skliper commented 5 years ago

Trac comment by abrown4 on 2015-07-20 16:30:59:

commit: [changeset:0ca6b09] Fixed osloader OS_ModuleLoad() bug when module_name too long. (Trac #98)

skliper commented 5 years ago

Trac comment by abrown4 on 2015-07-20 16:48:17:

commit: [changeset:17a4c30] Trac #100, Fixed vxworks OS_SymTableIterator() name arg too long.

skliper commented 5 years ago

Trac comment by abrown4 on 2015-07-20 17:20:09:

commit: [changeset:8e3608d] Trac #101, Fixed OS_SymTableIterator() unchecked write(). Added a unit test to confirm fix. Updated hardcoded values in other unit tests (no change in behavior or coverage).

skliper commented 5 years ago

Trac comment by abrown4 on 2015-07-20 19:00:15:

commit: [changeset:803fe97] Trac #105, Fixed OS_GetErrorName() missing codes. Also fixed the incorrect return for OS_ERROR_ADDRESS_MISALIGNED.

skliper commented 5 years ago

Trac comment by dasp on 2015-07-23 11:46:06:

commit: [changeset:0c91d99] Initial commit of UtAssert tests for osfilesys.c commit: [changeset:0ac78a6] Updated UtAssert tests for osfilesys.c and files for developing tests in Linux

skliper commented 5 years ago

Trac comment by jphickey on 2015-07-23 11:51:38:

Please note again that there is UTassert framework and filesystem tests on the mainline development branch. The last commit appears to be linux-targeted and any changes here really should be on the mainline branch. (I was under the impression this branch was Vxworks-specific class A stuff only)

skliper commented 5 years ago

Trac comment by abrown4 on 2015-07-23 12:46:59:

Replying to [comment:41 jphickey]:

Please note again that there is UTassert framework and filesystem tests on the mainline development branch. The last commit appears to be linux-targeted and any changes here really should be on the mainline branch. (I was under the impression this branch was Vxworks-specific class A stuff only)

FYI, the commits here in the OSAL src/unit-tests-coverage/osfilesys-test handle two separate issues: 1) the unit test .c's & .h's used for the VxWorks OSAL coverage (_wb) tests 2) a Makefile and fakes/stubs for use on linux, where we can stub out and develop the tests much more quickly than for the VxWorks target.

We're committing the extra linux Makefiles & linux fakes so that we can share them and run each others' tests on linux first for development/debugging. This has been an immense time-saver. Sure, the organization may need work, but we can work on directory organization after we get the tests all up and going.

skliper commented 5 years ago

Trac comment by jphickey on 2015-07-23 12:58:01:

Understood that it is easier to run fakes/stubs on Linux - my main concern is that I see unit test code that is going to conflict/duplicate/overlap stuff that is already on mainline development.

Has there been any consideration to rebase/merge this stuff with development? There are quite a few commits on this branch and I would really like to see some of them get merged in so we can start using these improvements. I believe this would also benefit your development efforts in that you would not need to re-do this unit test stuff and you could leverage what is already there, too.

FYI - WRT VxWorks, very very little has changed between {{{master}}} and {{{development}}} at least within the real implementation part i.e. the {{{src/os/vxworks}}} directory.

skliper commented 5 years ago

Trac comment by glimes on 2015-07-23 14:02:09:

OS_ShellOutputToFile case 4 in osfile is fragile: if "osapi.o" does not exist in the current working directory when the test program is executed, the test fails (and fails to tell us why).

This will break when we stage the test to a test target machine.

Because I suspect this isn't going to be high on your priority list, I will augment my test setup scripts to work around the problem.

I mention this because ... well, that's the only real test failure that I find when I run the results of merging trac-22 with development on the 32-bit X86 PC-Linux test target.

Oh, did I mention that I'm successfully building the merge of trac-22 with development, and it is going fairly well? :) ;)

[edit below: removed changeset links as they are changing too rapidly for a changeset link to be useful.]

So yes, I have a Bamboo plan branch (and an OSAL branch) that now successfully run your tests, and a second set that runs the merge of your branch with development. While I still have to update those branches manually having these available should be helpful.

Your commit pace is fast enough that it seemed like a good idea to get the bulk of the merge work done in advance, and to try to track your updates periodically, so that when you are happy with the set of tests you need for OSAL 4.1.1 we can bring them over to development without months of merge pain ;)

Current reference information for the test branches and Bamboo plans, as of last night's scheduled build:

babelfish/ic-gll-jsc-update This branch contains ephemeral changes I need to apply so that my existing scripts can build and run your tests. This branch will be rebased and amended, so ''don't branch from it''. This commit will descend from a recent commit on the trac-22 branch, usually the one at the tip when I do my update.

Sorry, no changeset link -- this is getting rebased and amended so frequently that a changeset link would not be useful.

https://babelfish.arc.nasa.gov/bamboo/browse/CFS-CFSOSAL60/latest This is where you find the latest build results for my slightly edited version of your branch. (wow, we've burned through 60 plan branches already!)

This builds and tests with the classic build mechanism.

It is possible to examine the Bamboo logs to find out exactly which commit was used. If we have to do this often, I will figure out a way to make it easier.

babelfish/ic-gll-jsc-merge This is the merge of your branch with development (it does not include the workaround branch mentioned above). For the record, I used a tool called "git imerge" in combination with a meld hack that helps resolve conflicts involving end-of-line changes.

Sorry, no changeset link -- this is getting rebased and amended so frequently that a changeset link would not be useful.

This will usually merge from the same commit of trac-22 used as the basis for the update branch.

This also merges in changes I need to make to development in order to bring "classic build and test" back to a working condition.

This branch may have inconsistant end-of-line markers, inconsistant whitespace, and an inconsistant idea of the width of a tab, as it is intended to be as close to a "mechanical merge" as possible (to make future merges easy).

As above, this branch will be rebased and amended, so ''don't branch from it''.

https://babelfish.arc.nasa.gov/bamboo/browse/CFS-CFSOSAL58/latest This is where you find the latest build results for this branch.

This builds and tests with the classic build '''and''' the CMake system.

It is possible to examine the Bamboo logs to find out exactly which commit was used. If we have to do this often, I will figure out a way to make it easier.

skliper commented 5 years ago

Trac comment by dasp on 2015-07-23 15:16:21:

commit: [changeset:0f36fb6] Minor updates to compile osfilesys.c tests on VxWorks

skliper commented 5 years ago

Trac comment by abrown4 on 2015-07-24 15:35:19:

commit: [changeset:67e90ac] Trac #104, fixed OS_Milli2Ticks() out of range bugs. commit: [changeset:495f229] Trac #45, osfile_wb test bugfixes for VxWorks execution. commit: [changeset:301a46a] Trac #45, fixed the osfile_wb test hanging on vxworks.

skliper commented 5 years ago

Trac comment by abrown4 on 2015-07-24 18:39:17:

Merge for ticket #103 onto branch #45. These changes fix everything except for the TimedWait() and SemTake() -type functions.

commit: [changeset:1ab6906] Trac #103, fixed osapi.c task semaphore usage.

commit: [changeset:6b37df9] Trac #103, fixed osapi.c queue semaphore usage (except for OS_QueuePut() ).

commit: [changeset:d19e229] Trac #103, fixed osapi.c binary semaphore usage, except for OS_BinSemTimedWait().

commit: [changeset:a18e1d0] Trac #103, fixed osapi.c queue semaphore usage in OS_QueuePut().

commit: [changeset:be741ce] Trac #103, Continue osapi.c semaphore fixes. Fixes to OS_QueuePut(), OS_BinSemGive(), OS_BinSemFlush(). Rolling back changes to: OS_BinSemTake(), OS_BinSemTimedWait() pending design decision.

commit: [changeset:2150e71] Trac #103, updated unit tests for binary semaphores.

commit: [changeset:c527718] Trac #103, osapi.c counting semaphore fixes (with exceptions). Note: OS_CountSemTake() and OS_CountSemTimedWait() are not protected.

commit: [changeset:596f739] Trac #103, osapi.c mutex semaphore table fixes (with an exception) Note: OS_MutSemTake() was not changed.

skliper commented 5 years ago

Trac comment by abrown4 on 2015-07-24 18:53:38:

commit: [changeset:0383f17] Trac #104, Fixed osapi.c OS_Milli2Ticks() to return OS_ERROR. Also removed hardcoded number in favor of including limits.h.

skliper commented 5 years ago

Trac comment by abrown4 on 2015-07-24 20:13:43:

commit: [changeset:41d0208] Trac #45, added osfilesys-test to linux Makefile. commit: [changeset:d114775] Trac #45, linux osfilesys-test/Makefile writes to log. commit: [changeset:d642ff9] Trac #45, Made oscore_wb testcase have similar stats on all platforms. The number of tests being run/pass/fail were slightly different. (Now they're identical, useful for debugging.)

skliper commented 5 years ago

Trac comment by abrown4 on 2015-07-24 20:52:15:

Replying to [comment:45 dasp]:

commit: [changeset:0f36fb6] Minor updates to compile osfilesys.c tests on VxWorks

Identified issues #108, #109, #110, #111, #112, #113, #114, & #115 for osfilesys.c thanks to Alan's hard work. Whew!

skliper commented 5 years ago

Trac comment by abrown4 on 2015-07-27 20:09:51:

commit: [changeset:a0f957a] Trac #103, osfileapi.c semaphore locking fixes & rollbacks. Rolled back many of the simple table accesses for risk reduction. Added selected lock/unlock pairs for functions that may block or yield during table modifications.

skliper commented 5 years ago

Trac comment by abrown4 on 2015-07-28 14:58:37:

commit: [changeset:270c2dc] Trac #92, Removed table locks in ostimer.c OS_TimerSignalHandler(). Locking a global table while calling a non-osal callback isn't a good idea.