Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

update OSAL for Class A & Associated unit tests #45

Closed
skliper opened this issue Sep 30, 2019 · 61 comments
Closed

update OSAL for Class A & Associated unit tests #45

skliper opened this issue Sep 30, 2019 · 61 comments
Labels
Milestone

Comments

@skliper
Copy link
Contributor

skliper commented Sep 30, 2019

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 skliper added this to the osal-4.2 milestone Sep 30, 2019
@skliper skliper self-assigned this Sep 30, 2019
@skliper skliper added the bug label Sep 30, 2019
@skliper
Copy link
Contributor Author

skliper commented Sep 30, 2019

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

@skliper
Copy link
Contributor Author

skliper commented Sep 30, 2019

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
Copy link
Contributor Author

skliper commented Sep 30, 2019

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
}}}

  • The OS_GetErrorName failure is from ut_oscore_log.txt
  • The OS_mv failure is from ut_osfile_log.txt
  • The other four are from ut_osfilesys_log.txt

@skliper
Copy link
Contributor Author

skliper commented Sep 30, 2019

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
Copy link
Contributor Author

skliper commented Sep 30, 2019

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
Copy link
Contributor Author

skliper commented Sep 30, 2019

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
Copy link
Contributor Author

skliper commented Sep 30, 2019

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:

  • Updating all OSAL files to comply with "most" MISRA rules and cppcheck static analysis errors
  • Updating all OSAL files to remove compiler warnings
  • Expanding existing unit tests to provide full API-level test coverage based on exposed uncovered code
  • Adding a full ADDITIONAL unit test suite using UTAssert framework to provide full code and BRANCH level coverage of the OSAL source code.

Subsequent commits should address these issues.

@skliper
Copy link
Contributor Author

skliper commented Sep 30, 2019

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
Copy link
Contributor Author

skliper commented Sep 30, 2019

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
Copy link
Contributor Author

skliper commented Sep 30, 2019

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
Copy link
Contributor Author

skliper commented Sep 30, 2019

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
Copy link
Contributor Author

skliper commented Sep 30, 2019

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
Copy link
Contributor Author

skliper commented Sep 30, 2019

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

[changeset:7376f89] Added

@skliper
Copy link
Contributor Author

skliper commented Sep 30, 2019

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

Commit: [changeset:05a6a3f] More MISRA

@skliper
Copy link
Contributor Author

skliper commented Sep 30, 2019

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
Copy link
Contributor Author

skliper commented Sep 30, 2019

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
Copy link
Contributor Author

skliper commented Sep 30, 2019

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
Copy link
Contributor Author

skliper commented Sep 30, 2019

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
Copy link
Contributor Author

skliper commented Sep 30, 2019

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
Copy link
Contributor Author

skliper commented Sep 30, 2019

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
Copy link
Contributor Author

skliper commented Sep 30, 2019

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
Copy link
Contributor Author

skliper commented Sep 30, 2019

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
Copy link
Contributor Author

skliper commented Sep 30, 2019

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
Copy link
Contributor Author

skliper commented Sep 30, 2019

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
Copy link
Contributor Author

skliper commented Sep 30, 2019

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
Copy link
Contributor Author

skliper commented Sep 30, 2019

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

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

@skliper
Copy link
Contributor Author

skliper commented Sep 30, 2019

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.

This was referenced Sep 30, 2019
@skliper skliper removed their assignment Sep 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant