Skip to content

Commit

Permalink
Fix #32, compile out CF_Asserts by default
Browse files Browse the repository at this point in the history
Changes CF_Assert to be opt-in rather than opt-out, so that under normal
verification and validation the asserts will _not_ be included, but they
can still be added back during development, if desired.

They mainly exist as notes to developers as to what is supposed to be true,
once debugged it is impossible to get these conditions, by definition.

Also removes one channel calculation that was only for assert.  Note that
the same condition is asserted later, so it was redundant anyway.
  • Loading branch information
jphickey committed Jan 6, 2022
1 parent b6de205 commit 743aaf3
Show file tree
Hide file tree
Showing 9 changed files with 25 additions and 225 deletions.
1 change: 0 additions & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ include_directories(${CFS_IO_LIB_MISSION_DIR}/fsw/public_inc)

set(APP_SRC_FILES
fsw/src/cf_app.c
fsw/src/cf_assert.c
fsw/src/cf_cfdp.c
fsw/src/cf_cfdp_r.c
fsw/src/cf_cfdp_s.c
Expand Down
35 changes: 0 additions & 35 deletions fsw/src/cf_assert.c

This file was deleted.

42 changes: 24 additions & 18 deletions fsw/src/cf_assert.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,27 +29,33 @@

#include "cfe.h"

extern void CF_HandleAssert(const char *file, int line);

/*
* Note that in some cases, code in CF may compute or store a value for the
* sole purpose of checking it with a CF_Assert(). If CF_Assert is then entirely
* compiled out with NDEBUG, the compiler may see that as an unused value and
* trigger a warning.
*
* To avoid this, a no-op inline function is used, such that the value in the
* CF_Assert call is still evaluated, but the result is ignored.
* CF_Assert statements within the code are primarily informational for developers,
* as the conditions within them should always be true. Barring any unforeseen
* bugs in the code, they should never get triggered. However, if the code is
* modified, these conditions could happen, so it is still worthwhile to keep
* these statements in the source code, so they can be enabled if necessary.
*/

#ifdef NDEBUG
/* this is release mode */
static inline void CF_NoAssert(bool cond)
{
/* no-op to avoid unused value warning */
}
#define CF_Assert(x) CF_NoAssert(x)
#else /* NDEBUG */
#ifdef CF_DEBUG_BUILD

/*
* Debug build:
* Translate CF_Assert to the system assert. Note that asserts may still get disabled
* if building with NDEBUG flag set, even if CF_DEBUG_BUILD flag is enabled.
*/
#include <assert.h>
#define CF_Assert(x) assert(x)
#endif /* !NDEBUG */

#else /* CF_DEBUG_BUILD */

/*
* Normal build:
* It should be impossible to get any conditions which are asserted, so it should
* be safe to turn these off. This is the configuration that the code should be
* normally tested and verified in.
*/
#define CF_Assert(x) /* no-op */

#endif /* CF_DEBUG_BUILD */
#endif /* !CF_ASSERT__H */
5 changes: 1 addition & 4 deletions fsw/src/cf_utils.c
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,7 @@ CF_Transaction_t *CF_FindUnusedTransaction(CF_Channel_t *c)

if (c->qs[CF_QueueIdx_FREE])
{
int q_index; /* initialized below in if */
const int chan_index = (c - CF_AppData.engine.channels);
int q_index; /* initialized below in if */

CF_CListNode_t *n = c->qs[CF_QueueIdx_FREE];
CF_Transaction_t *t = container_of(n, CF_Transaction_t, cl_node);
Expand All @@ -66,8 +65,6 @@ CF_Transaction_t *CF_FindUnusedTransaction(CF_Channel_t *c)
/* now that a transaction is acquired, must also acquire a history slot to go along with it */
if (c->qs[CF_QueueIdx_HIST_FREE])
{
CF_Assert(CF_AppData.hk.channel_hk[chan_index].q_size[CF_QueueIdx_HIST] <
CF_NUM_HISTORIES_PER_CHANNEL); /* sanity check */
q_index = CF_QueueIdx_HIST_FREE;
}
else
Expand Down
2 changes: 0 additions & 2 deletions unit-test/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,6 @@ add_cfe_coverage_stubs(cf_internal

stubs/cf_app_global.c
stubs/cf_app_stubs.c
stubs/cf_assert_handlers.c
stubs/cf_assert_stubs.c
stubs/cf_cfdp_handlers.c
stubs/cf_cfdp_dispatch_stubs.c
stubs/cf_cfdp_r_stubs.c
Expand Down
80 changes: 0 additions & 80 deletions unit-test/cf_assert_tests.c

This file was deleted.

3 changes: 0 additions & 3 deletions unit-test/cf_utils_tests.c
Original file line number Diff line number Diff line change
Expand Up @@ -305,7 +305,6 @@ void Test_cf_dequeue_transaction_Call_CF_CList_Remove_AndDecrement_q_size(void)
uint16 updated_q_size = CF_AppData.hk.channel_hk[arg_t.chan_num].q_size[arg_t.flags.com.q_index];

/* Assert */
UtAssert_STUB_COUNT(CF_HandleAssert, 0);
UtAssert_ADDRESS_EQ(context_clist_remove.head, expected_head);
UtAssert_ADDRESS_EQ(context_clist_remove.node, expected_cl_node);
UtAssert_True(updated_q_size == initial_q_size - 1, "q_size is %d and that is 1 less than initial value %d",
Expand Down Expand Up @@ -381,7 +380,6 @@ void Test_cf_move_transaction_Call_CF_CList_InsertBack_AndSet_q_index_ToGiven_q(
CF_MoveTransaction(arg_t, arg_q);

/* Assert */
UtAssert_STUB_COUNT(CF_HandleAssert, 0);
UtAssert_STUB_COUNT(CF_CList_Remove, 1);
UtAssert_ADDRESS_EQ(context_clist_remove.head, expected_remove_head);
UtAssert_ADDRESS_EQ(context_clist_remove.node, expected_remove_node);
Expand Down Expand Up @@ -447,7 +445,6 @@ void Test_CF_CList_Remove_Ex_Call_CF_CList_Remove_AndDecrement_q_size(void)
uint16 updated_q_size = CF_AppData.hk.channel_hk[arg_c - CF_AppData.engine.channels].q_size[arg_index];

/* Assert */
UtAssert_STUB_COUNT(CF_HandleAssert, 0);
UtAssert_STUB_COUNT(CF_CList_Remove, 1);
UtAssert_ADDRESS_EQ(context_clist_remove.head, expected_remove_head);
UtAssert_ADDRESS_EQ(context_clist_remove.node, expected_remove_node);
Expand Down
33 changes: 0 additions & 33 deletions unit-test/stubs/cf_assert_handlers.c

This file was deleted.

49 changes: 0 additions & 49 deletions unit-test/stubs/cf_assert_stubs.c

This file was deleted.

0 comments on commit 743aaf3

Please sign in to comment.