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.

Sign up for GitHub

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

Jump to bottom

Correcting NO_ASSERT handling and safety. Fixes #1706, #2447 #2448

Merged

LeStarch merged 1 commit into nasa:devel from LeStarch:issue-1706

Jan 5, 2024

Collaborator

LeStarch commented Dec 21, 2023


*Related Issue(s)*
*Has Unit Tests (y/n)*	y
*Documentation Included (y/n)*

Change Description

FW_NO_ASSERT was both broken (would not compile) and would introduce bugs into the code. This fixes these issues (#1706, #2447).

FW_NO_ASSERT does not remove definitions for AssertHook and associated functions
FW_NO_ASSERT now maintains side-effects from assertion code
F´ code updated to not perform side-effects in assertion code
Unit tests updated for FW_NO_ASSERT

Rationale

Bug fixes.

Future Work

Update coding standards.


          Correcting NO_ASSERT handling and safety. Fixes nasa#1706, nasa#2447

4dae21f

github-advanced-security bot found potential problems

View reviewed changes

Svc/GenericHub/GenericHubComponentImpl.cpp

-                  FW_ASSERT(serializer.serialize(id) == Fw::SerializeStatus::FW_SERIALIZE_OK);
-                  FW_ASSERT(serializer.serialize(timeTag) == Fw::SerializeStatus::FW_SERIALIZE_OK);
-                  FW_ASSERT(serializer.serialize(val) == Fw::SerializeStatus::FW_SERIALIZE_OK);
+                  status = serializer.serialize(id);

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter id has not been checked.

Svc/GenericHub/GenericHubComponentImpl.cpp

-                  FW_ASSERT(serializer.serialize(val) == Fw::SerializeStatus::FW_SERIALIZE_OK);
+                  status = serializer.serialize(id);
+                  FW_ASSERT(status == Fw::SerializeStatus::FW_SERIALIZE_OK);
+                  status = serializer.serialize(timeTag);

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter timeTag has not been checked.

Svc/GenericHub/GenericHubComponentImpl.cpp

+                  FW_ASSERT(status == Fw::SerializeStatus::FW_SERIALIZE_OK);
+                  status = serializer.serialize(timeTag);
+                  FW_ASSERT(status == Fw::SerializeStatus::FW_SERIALIZE_OK);
+                  status = serializer.serialize(val);

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter val has not been checked.

Svc/GenericHub/GenericHubComponentImpl.cpp

-                  FW_ASSERT(serializer.serialize(timeTag) == Fw::SerializeStatus::FW_SERIALIZE_OK);
-                  FW_ASSERT(serializer.serialize(severity) == Fw::SerializeStatus::FW_SERIALIZE_OK);
-                  FW_ASSERT(serializer.serialize(args) == Fw::SerializeStatus::FW_SERIALIZE_OK);
+                  status = serializer.serialize(id);

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter id has not been checked.

Svc/GenericHub/GenericHubComponentImpl.cpp

-                  FW_ASSERT(serializer.serialize(args) == Fw::SerializeStatus::FW_SERIALIZE_OK);
+                  status = serializer.serialize(id);
+                  FW_ASSERT(status == Fw::SerializeStatus::FW_SERIALIZE_OK);
+                  status = serializer.serialize(timeTag);

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter timeTag has not been checked.

Svc/GenericHub/GenericHubComponentImpl.cpp

+                  FW_ASSERT(status == Fw::SerializeStatus::FW_SERIALIZE_OK);
+                  status = serializer.serialize(timeTag);
+                  FW_ASSERT(status == Fw::SerializeStatus::FW_SERIALIZE_OK);
+                  status = serializer.serialize(severity);

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter severity has not been checked.

Svc/GenericHub/GenericHubComponentImpl.cpp

+                  FW_ASSERT(status == Fw::SerializeStatus::FW_SERIALIZE_OK);
+                  status = serializer.serialize(severity);
+                  FW_ASSERT(status == Fw::SerializeStatus::FW_SERIALIZE_OK);
+                  status = serializer.serialize(args);

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter args has not been checked.

github-advanced-security bot found potential problems

View reviewed changes

Os/Posix/IntervalTimer.cpp

@@ @@ -14,8 +14,8 @@ @@
               namespace Os {
                   void IntervalTimer::getRawTime(RawTime& time) {
                       timespec t;
-                      FW_ASSERT(clock_gettime(CLOCK_REALTIME,&t) == 0,errno);
+                      PlatformIntType status = clock_gettime(CLOCK_REALTIME,&t);

Check notice

Code scanning / CodeQL

Use of basic integral type Note

status uses the basic integral type int rather than a typedef with size and signedness.

LeStarch requested a review from timcanham

December 21, 2023 22:03

LeStarch requested a review from thomas-bc

January 4, 2024 17:57

thomas-bc approved these changes

View reviewed changes

LeStarch merged commit 1caf1e4 into nasa:devel

34 checks passed

Collaborator

thomas-bc commented Jan 5, 2024

This change is likely going to trip off https://github.com/fprime-community/fprime-tools/blob/c43c85f47f6ae552c68897c456c2ca49bc5dcfcf/.github/workflows/integration-tests.yml#L38-L44 ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet