nasa / cf

The Core Flight System (cFS) CFDP application.

core Flight System (cFS) CFDP Application (CF)

Introduction

The CFDP application (CF) is a core Flight System (cFS) application that is a plug in to the Core Flight Executive (cFE) component of the cFS.

CF is a cFS application for providing CFDP (CCSDS File Delivery Protocol) CCSDS 727.0-B-5 compliant services. Its primary function is to provide file receive and transmit functionality to this protocol. It works by mapping CFDP PDUs on and off cFS's software bus.

The CF application is written in C and depends on the cFS Operating System Abstraction Layer (OSAL) and cFE components. There is additional CF application specific configuration information contained in the application user's guide.

User's guide information can be generated using Doxygen (from top mission directory):

  make prep
  make -C build/docs/cf-usersguide cf-usersguide

Software Required

cFS Framework (cFE, OSAL, PSP)

An integrated bundle including the cFE, OSAL, and PSP can be obtained at https://github.com/nasa/cfs

About cFS

The cFS is a platform and project independent reusable software framework and set of reusable applications developed by NASA Goddard Space Flight Center. This framework is used as the basis for the flight software for satellite data systems and instruments, but can be used on other embedded systems. More information on the cFS can be found at http://cfs.gsfc.nasa.gov

cf's Issues

Null pointer dereference in cf_callbacks.c

CF/fsw/src/cf_callbacks.c

Line 397 in cbda7b9

if(QueueEntryPtr->Preserve == CF_DELETE_FILE)

After the QueueEntryPtr != NULL check, the code continues with the check if (QueueEntryPtr->Preserve == CF_DELETE_FILE). If QueueEntryPtr is null, accessing QueueEntryPtr->Preserve causes etiher exception or invalid check.

if (Chan != `CF_ERROR)
{
	CF_AppData.Hk.Chan[Chan].SuccessCounter++;
	QueueEntryPtr = CF_FindPbNodeByTransNum(Chan, CF_PB_ACTIVEQ, TransInfo.trans.number);
	if (QueueEntryPtr != NULL)
	{
		QueueEntryPtr->Status = CF_STAT_SUCCESS;
	}
}

if (QueueEntryPtr->Preserve == CF_DELETE_FILE)
{
	OS_remove(&TransInfo.md.source_file_name[0]);
}  

Unreachable code block in check for CF_SEND_NO_MSG

This issue was imported from the GSFC issue tracking system

Imported from: [GSFCCFS-1775] Unreachable code block in check for CF_SEND_NO_MSG
Originally submitted by: Hickey, Joseph P. (GSFC-582.0)[VANTAGE SYSTEMS INC] on Tue Nov 16 14:19:04 2021

Original Description:
This issue was originally reported by IV&V, creating Jira issue to track its disposition and resolution.

The code within the IF block in cf_cfdp_s.c in the CF App source code, namely lines 158-159 will never be executed. The IF block checks variable 'status' if it is CF_SEND_NO_MSG on line 157, this variable originates from the return value of CF_CFDP_SendFd(), which can only return values CF_SEND_SUCCESS and CF_SEND_ERROR.

Improve commenting throughout CF v3.0

This issue was imported from the GSFC issue tracking system

Imported from: [GSFCCFS-1194] Improve commenting throughout CF v3.0
Originally submitted by: Timmons, Elizabeth J. (GSFC-5820) on Mon Jul 20 13:32:23 2020

Original Description:
CF v3.0 needs improved commenting throughout. Specifically:

• file comments in every file describing the purpose of the file
• explanations and limits for all configuration parameters
• ensure that all comments are accurate

Cleanup CF v3.0 Perf ID handling

This issue was imported from the GSFC issue tracking system

Imported from: [GSFCCFS-1190] Cleanup CF v3.0 Perf ID handling
Originally submitted by: Timmons, Elizabeth J. (GSFC-5820) on Fri Jul 17 13:22:51 2020

Original Description:
Encompasses several findings from the CF v3.0 code review:

• Remove unused perf id CF_DIRREAD_PERF_ID
• Determine whether perf ids CF_PDU_RCVD_PERF_ID and CF_VCxPDUSENT_PERF_ID should be added (currently used in stakeholder)
• Ensure that CFE_ES_PerfLogExit is called when the app exits

CF Doxygen Documentation Needs Updates

This issue was imported from the GSFC issue tracking system

Imported from: [GSFCCFS-1209] CF Doxygen Documentation Needs Updates
Originally submitted by: Timmons, Elizabeth J. (GSFC-5820) on Wed Jul 29 14:54:40 2020

Original Description:
The CF doxygen documentation is inaccurate for v3.0.

Unreachable code block in check for CF_SEND_ERROR

This issue was imported from the GSFC issue tracking system

Imported from: [GSFCCFS-1774] Unreachable code block in check for CF_SEND_ERROR
Originally submitted by: Hickey, Joseph P. (GSFC-582.0)[VANTAGE SYSTEMS INC] on Tue Nov 16 14:16:04 2021

Original Description:
This issue was originally reported by IV&V, creating Jira issue to track its disposition and resolution.

The code within the ELSE IF block in cf_cfdp_s.c in the CF App source code, namely lines 698-699 will never be executed. The ELSE IF block checks variable 'sret' if is is CF_SEND_ERROR on line 697, this value originates from the return value of CFDP_S_SendEof() which can only return values CF_SEND_SUCCESS and CF_SEND_NO_MSG.

CF method CF_CFDP_ReceiveMessage will always segfault on line 1289

This issue was imported from the GSFC issue tracking system

Imported from: [GSFCCFS-1734] CF method CF_CFDP_ReceiveMessage will always segfault on line 1289
Originally submitted by: Gibson, Alan S. (GSFC-5870) on Thu Sep 23 11:24:05 2021

Original Description:
Line 1289:
++CF_AppData.hk.channel_hk[t->chan_num].counters.recv.dropped;

The only path to get into this line runs through an if(t) failure, which means t will always be NULL at this point. If it is changed to just chan_num (a variable defined within the function) it will not fail. However, it is not known if that is the intent.

There is a previous if block that redefines the variable t, from a transaction_t* to a transaction_t, but its scope is only within that if block. Thus when we reach line 1289, t is the transaction_t* type and will always be NULL.

(link removed)

CF test passing uninitialized buffer to input-only parameter

This issue was imported from the GSFC issue tracking system

Imported from: [GSFCCFS-1783] CF test passing uninitialized buffer to input-only parameter
Originally submitted by: Hickey, Joseph P. (GSFC-582.0)[VANTAGE SYSTEMS INC] on Tue Nov 16 17:16:22 2021

Original Description:
The test function Test_CF_CFDP_CopyDataToLv_FailsBecause_len_IsEqTo_sizeof_dest_lv_data_Returns_neg1 calls 'CF_CFDP_CopyDataToLv' but passes an uninitialized buffer (arg_data) to the function when this parameter is documented as input only.

The test passes because the value happens to be a "don't-care" in this case, so it does not affect the outcome/operation, but its bad practice to pass uninitialized data to an input, and also some compilers will generate a warning about this too.

Interestingly, the call to 'AnyBufferOf_uint8_WithSize' which would have initialized this, is commented out. This may be a simple mistake - uncommenting this line will resolve the error.

CF Code Style and Coding standards compliance

This issue was imported from the GSFC issue tracking system

Imported from: [GSFCCFS-1778] CF Code Style and Coding standards compliance
Originally submitted by: Hickey, Joseph P. (GSFC-582.0)[VANTAGE SYSTEMS INC] on Tue Nov 16 16:40:53 2021

Original Description:
There are a number of style aspects of the CF source code that should be cleaned up to better comply with GSFC coding standards:

• inconsistent indentation/spaces, brace/comment style, lots of lines with trailing whitespace.
• inline variable instantiations
• variables and other symbols at global scope not properly named

Recommendations:

• Make a one-time pass through "clang-format" using same rules as applied to CFE framework to clean up whitespace
• All local variables should be declared at function start
• All global variables should be qualified with the app prefix (e.g. CF_) and ideally all application state should be consolidated into a single top-level global variable (such that it can be memset to zero if/when application restarts).

CF function CF_CFDP_IsSender(transaction_t *ti) is odd one out because it uses ti

This issue was imported from the GSFC issue tracking system

Imported from: [GSFCCFS-1728] CF function CF_CFDP_IsSender(transaction_t *ti) is odd one out because it uses ti
Originally submitted by: Gibson, Alan S. (GSFC-5870) on Tue Sep 14 10:58:59 2021

Original Description:
Every other function in cfdp.c that uses a transaction_t* as an argument names it 't', but CF_CFDP_IsSender uses ti.

Review use of 'unlikely'

This issue was imported from the GSFC issue tracking system

Imported from: [GSFCCFS-1236] Review use of 'unlikely'
Originally submitted by: Gibson, Alan S. (GSFC-5870) on Fri Aug 28 17:49:32 2020

Original Description:
In cf_assert.h the NDEBUG #ifdef runs in 'BUILDTYPE=release' mode which replaces CF_Assert with
    if(unlikely((x))) CF_HandleAssert(__FILE__, __LINE__);
This line is incorrect, because the other path uses
    assert(x)
However, assert does nothing when true, the unlikely path causes assert when true. This needs updated with (!x)

Also, there are calls to CF_Assert using "short-circuit" Boolean statements, these do not "short-circuit" because of the unlikely.

unlikely is most commonly defined as:
    __builtin_expect(!!(x), 0)

CFDP setup between two cFS projects

Hi,
I have setup two cFS projects on a Linux VM with CF running on each one with CI and TO to test file transfer between the two projects as shown below:

When I try to transfer file /cf/cf.so from one side to the other (destination file name /cf/cf_sent.so) I get the following result (file sent from ppesim-cfs to pdpmep-fsw):

When I change the code at cf_app.c line 1030 from:

To:

The received packet's length is shown to be correct but the file still doesn't make it across.
If I attempt to run the unchange CF code between the Linux VM (little endian machine) and a powerPC (big endian) I get a segmentation fault (most likely because the powerPC is running the cFS project as root which I confirmed by running the receiving end of the VM version as sudo and I got a segmentation fault as well).

To start, is there anything I am missing with setting up two CF applications this way? Or is there any more information you might need me to provide?

Thanks,
Ashraf.

Use of Playback Directory command

I'd expect the Playback Directory command to function like the following. Assume 7 is the maximum number of transactions that can be occurring simultaneously and that the directory we're attempting to playback has 10 files in it:

• All of the files in the directory get queued.
• Transactions are started for 7 files, at which point the maximum number of transactions is reached.
• Once 1 transaction finishes, CF reads from the queue and starts a transaction for the 8th file.
• Continue until the entire directory has been downlinked.

However, during our testing the command is functioning like the following:

• All of the files in the directory get queued.
• Transactions are successfully started for 7 files, at which point the maximum number of transactions is reached.
• CF also attempts to start transactions for the remaining 3 files at the same time, but they error out since we're at the maximum number of transactions. The remaining files are never downlinked.

I'm not sure if we're misunderstanding how the command is intended to be used or if there is a bug.

CF method CF_CList_Remove appears to accept bad arguments

This issue was imported from the GSFC issue tracking system

Imported from: [GSFCCFS-1719] CF method CF_CList_Remove appears to accept bad arguments
Originally submitted by: Gibson, Alan S. (GSFC-5870) on Thu Aug 26 13:23:38 2021

Original Description:
A bad node is passed to CF_CList_Remove, but it carries on unaware. This was found because branch 3 of if((node->next==node)&&(node->prev==node)) can only be covered by a test that passes this situation, node->next == node, node->prev != node.

The issue: should a single node enter, that has a node->next == node (meaning: pointing to itself), but a node->prev != node (meaning: NOT pointing to itself), the code continues as if this is a valid state. It may be impossible for a bad node to enter here, but that is not apparent at the unit test level for CF_CList_Remove nor is the Doxygen brief clear on that assumption.

CF unit tests use incorrect dummy buffers

This issue was imported from the GSFC issue tracking system

Imported from: [GSFCCFS-1780] CF unit tests use incorrect dummy buffers
Originally submitted by: Hickey, Joseph P. (GSFC-582.0)[VANTAGE SYSTEMS INC] on Tue Nov 16 16:59:31 2021

Original Description:
The CF unit tests contain an oft-repeated sequence to initialize a message pointer, for example:

    /* Arrange */
    pdu_r_msg_t dummy_ph;
    int local_result;

    CF_AppData.engine.in.msg = (CFE_SB_Buffer_t*)&dummy_ph;

This sequence is not valid for two reasons:

1. Because the pdu_r_msg_t instance is not aligned appropriately to be cast to a CFE_SB_Buffer_t*. This invalid cast generates a warning on many compilers.

2. Because the pdu_r_msg_t instance does not contain any additional data beyond the pdu_header_t value. Almost all CF calls will read beyond this header, depending on what the function call is, and some will write too. In the case of writing, this results in stack smashing, and the test may segfault/crash.

Recommendation is to create a union for the message buffer, which can address the alignment problem and also be used to reserve some extra space for data beyond the header that many calls do access.

It looks like this was done at one point in the "pdu_t" type (in cfdp.h) but this was commented out. Recommend reinstating this and using it in unit tests as it will be more correct.

CF method CF_CFDP_SendNak ph value can never be NULL

This issue was imported from the GSFC issue tracking system

Imported from: [GSFCCFS-1689] CF method CF_CFDP_SendNak ph value can never be NULL
Originally submitted by: Gibson, Alan S. (GSFC-5870) on Fri Jul 30 17:27:32 2021

Original Description:
The CF_CFDP_SendNak function receives it's header from &((pdu_s_msg_t*)CF_AppData.engine.out.msg)->ph but even when CF_AppData.engine.out.msg == NULL, ph is 0x10.

The check if ph is NULL (if (!ph)) will never be able to fire.

The check should be changed to see if CF_AppData.engine.out.msg is NULL instead; this will ensure that if/when it is NULL a ph value of 0x10 will not be used.

Note: This is probably not possible in the field, a "nack" should never be able to be returned without msg being populated, but in that case there is no reason to verify ph is not NULL.

CF Purge command does not appear to be hooked in

This issue was imported from the GSFC issue tracking system

Imported from: [GSFCCFS-1701] CF Purge command does not appear to be hooked in
Originally submitted by: Timmons, Elizabeth J. (GSFC-5820) on Tue Aug 10 17:30:05 2021

Original Description:
CF comments make reference to a purge command, and functions exist for that command. However, the command code referenced in the comments (CF_PURGE_QUEUE_CC) does not appear to exist and there does not appear to be an equivalent one.

(link removed)

(link removed)

CF test incorrect initialization of "dummy_channel" pipe ID

This issue was imported from the GSFC issue tracking system

Imported from: [GSFCCFS-1784] CF test incorrect initialization of "dummy_channel" pipe ID
Originally submitted by: Hickey, Joseph P. (GSFC-582.0)[VANTAGE SYSTEMS INC] on Tue Nov 16 17:22:51 2021

Original Description:
These three tests utilize a "dummy_channel" structure:

1. Test_CF_CFDP_DisableEngine_ClosesAllActiveFilesAndNoOpenPlaybackDirectoriesResetsAllQueueCountersDeletesPipe
2. Test_CF_CFDP_DisableEngine_ClosesAllActiveFilesAndAnyOpenPlaybackDirectoriesResetsAllQueueCountersDeletesPipe
3. Test_CF_CFDP_DisableEngine_ClosesAllActiveFilesAndAllOpenPlaybackDirectoriesResetsAllQueueCountersDeletesPipe

These are calling Any_uint8() and assigning that value to to the pipe member of the structure. However on Caelum the Pipe IDs are actually 32 bits like other IDs (not 8) and when using strict ID types, this assignment will (correctly) fail with a compiler error as being invalid.

The test needs to be explicit that it is intentionally using an integer as a made-up PipeID and needs to use the conversion macro do to so.

CF is incompatible with recent OSAL API changes

The cf_playback.c file calls the OS_opendir, OS_readdir, and OS_closedir functions, which were removed in favor of the OS_DirectoryOpen, OS_DirectoryRead, and OS_DirectoryClose functions. This is regarding OSAL 5.0.3-bv (git tag). I expect to see other issues as I make fixes to pass unit testing. I will post them here.

compile Errors with latest cFS

It appears that the changes made in nasa/cFE@4652c0d result in CF no longer compiling due to depreciated code being removed.

I am working on altering the CF source to get it to compile with these changes, but wanted to ensure that this was not already in progress.

CF Purge Queue Command Opcode Not Defined

This issue was imported from the GSFC issue tracking system

Imported from: [GSFCCFS-1765] CF Purge Queue Command Opcode Not Defined
Originally submitted by: Maldonado, Sergio E. (GSFC-580.0)[Arctic Slope Technical Services, Inc.] on Fri Oct 29 11:03:57 2021

Original Description:
The command opcode for Purge Queue is not present in the CF_CMDS enumeration in cf_msg.h. It should be present with a value of 21. The command dispatch table in cf_cmd.c does have an entry for the command, as well as the implementation. Without the opcode defined, the command cannot be verified at the functional level.

Playback Directory Command - timing bug

Related to issue #11

The playback queue is starting to process a new file after the End-of-File (EOF) PDU is sent for an old transaction, while the old transaction isn’t truly finished (and the ‘active transactions’ count decremented) until the ACK-FIN PDU is received. Due to this, one or more transactions can still be active waiting for their ACK-FIN PDU to be received while the playback queue starts a new transaction. If I had to guess, this is a relic of class1 (unacknowledged) transactions which end after the EOF PDU is sent.

This issue must have gone unnoticed for so long because in most cases it is hidden, as long as your files are reasonably large and your maximum number of simultaneous transactions is greater than 1 (ours is 7). This is because as long as the file is reasonably large, it will take more time to downlink the new file than it will take for the ACK-FIN from the old transaction to be received and fully closed out. Since the playback queue only transfers one file at a time, you’ll only have a maximum of two transactions active at once.

With maximum number of simultaneous transactions set to 7, if you try to download 8 small files via the playback directory command we encounter the issue (and is the case which found this bug in the first place). This is because the files are so small that 7 separate files can send all of their data and hit the ‘EOF’ phase before the first transaction receives its ‘ACK-FIN’ and fully closes out.

Even if the files are quite large, if I set the maximum number of simultaneous transactions to 1 the issue is encountered every time and the second file will fail to download. This is because a new transaction always attempts to start while the old transaction was between the ‘EOF Sent’ and ‘ACK-FIN received’ phases.

Would it be valuable for me to submit a bug fix to this repository? Unsure if this repository is monitored.

CF method CF_Assert call branches cannot be tested during unit testing

This issue was imported from the GSFC issue tracking system

Imported from: [GSFCCFS-1733] CF method CF_Assert call branches cannot be tested during unit testing
Originally submitted by: Gibson, Alan S. (GSFC-5870) on Wed Sep 22 16:31:48 2021

Original Description:
Calls to CF_Assert() cannot be stubbed by unit testing.
The reasons:

1. CF_Assert uses assert which will kill the running process and subsequently the unit test runner when called by code under test.
2. CF_Assert can be stubbed
       a. The stub cannot stop the return to the code under test.
       b. The value required to cover the branch will cause a segfault upon return -- as the intent of CF_Assert is to kill the app before it takes cFS with it.
       c. There is currently no allowable way to have these branches tested during automated runs.
       d. It is possible to run ad hoc tests to show the assert occurs, but the unit test will stop and not return to the original test code as the automated tests do.

CF CF_CFDP_MsgOutGet method double checks msg for NULL

This issue was imported from the GSFC issue tracking system

Imported from: [GSFCCFS-1727] CF CF_CFDP_MsgOutGet method double checks msg for NULL
Originally submitted by: Gibson, Alan S. (GSFC-5870) on Mon Sep 13 09:59:14 2021

Original Description:
The function CF_CFDP_MsgOutGet in cf_cfdp.c checks if(!CF_AppData.engine.out.msg) (line 356) as a top if block; however, it then checks it again as part of the if block
if(!CF_AppData.engine.out.msg&&((CF_AppData.config_table->chan[t->chan_num].sem_name[0]&&                     (OS_CountSemTimedWait(c->sem_id, 0)==OS_SUCCESS))||                     (!CF_AppData.config_table->chan[t->chan_num].sem_name[0])))
(starting on line 365 <- yep not a typo line 356 and 365, funny), but there is nothing in between that could change it.

It would appear that this check in the second if is not required.

request for configurable transaction packet MID

In our use case, we want to use CFDP to send files from CPU1 to CPU2 and from CPU1 to Ground. The two CPUs are connected via SBN, and the connection between CPU1 and the ground station is through TO and CI.

I had thought to use the MID of CFDP packets to filter which packets are let through TO and SBN. By setting up CF with two input channels and two output channels, each using separate MIDs, I could set TO to only listen for packets from one channel and SBN to listen for packets on the other CF channel.

It looks like all CFDP transfer packets use the CF_TRANS_TLM_MID, and that’s not configurable. Would it be possible to add a configuration value in cf_cfgtable to set MID for output channels, similar to what is done for the input channels?

Alternatively, is there a more standard way to do what I'm attempting?

CF_TraverseHistory string buffer handling

This issue was imported from the GSFC issue tracking system

Imported from: [GSFCCFS-1776] CF_TraverseHistory string buffer handling
Originally submitted by: Hickey, Joseph P. (GSFC-582.0)[VANTAGE SYSTEMS INC] on Tue Nov 16 14:22:27 2021

Original Description:
This issue was originally reported by IV&V, creating Jira issue to track its disposition and resolution.

Function CF_TraverseHistory() in cf_utils.c in the CF App source code, writes some text to the buffer 'linebuf' on line 70. This buffer, however, is overwritten on line 74 before it is written to the file descriptor on line 75. It is questionable whether this was the desired behavior.

CF function CF_CFDP_ProcessPlaybackDirectory has an oddly bracketed block

This issue was imported from the GSFC issue tracking system

Imported from: [GSFCCFS-1732] CF function CF_CFDP_ProcessPlaybackDirectory has an oddly bracketed block
Originally submitted by: Gibson, Alan S. (GSFC-5870) on Wed Sep 22 09:22:05 2021

Original Description:
In cfdp.c the function CF_CFDP_ProcessPlaybackDirectory has a block of code with no if/while et. Just a bracketed block of code that does not seem to have a reason to be bracketed.

CF source code needs format/whitespace scrub

In the CF-3.0.0 release candidate, the source code files have many formatting discrepancies from the CFE/CFS recommendations.

Before continuing development with other fixes, the source should be sent through the "clang-format" tool using the same rules as used for the CFE framework. This will establish a baseline for fixing other issues reported by CF users thus far.

Since this is likely to affect many lines, it must be done as an isolated commit, before any other code changes are done, as it will not be easy to merge/rebase with this type of change.

CF method CF_CFDP_MsgOutGet returns 0x10 when it should be NULL

This issue was imported from the GSFC issue tracking system

Imported from: [GSFCCFS-1726] CF method CF_CFDP_MsgOutGet returns 0x10 when it should be NULL
Originally submitted by: Gibson, Alan S. (GSFC-5870) on Fri Sep 10 14:28:59 2021

Original Description:
CF_CFDP_MsgOutGet has a path where it will return a 0x10 if the CF_AppData.engine.out.msg is NULL. There are two if blocks after the msg NULL verification that if neither come back true the assignment on line 382 of cf_cfdp.c:

ret = &((pdu_s_msg_t*)CF_AppData.engine.out.msg)->ph;

will make ret == 0x10, not NULL.
The description of the return statement shows that this is not correct:

\retstmt Pointer to a pdu_header_t within a software bus buffer on success. Otherwise NULL.

CF tests assume "assert" is available but do not include assert.h

This issue was imported from the GSFC issue tracking system

Imported from: [GSFCCFS-1788] CF tests assume "assert" is available but do not include assert.h
Originally submitted by: Hickey, Joseph P. (GSFC-582.0)[VANTAGE SYSTEMS INC] on Tue Nov 16 18:48:45 2021

Original Description:
Some CF test utility functions call "assert" on various items (e.g. Any_file_directive_t_Except).

Several issues with this:

• The "assert.h" system header was not included
• The condition "ERROR_RETRIEVING_ANY_VALUE" is a constant that is not even 0, so the assert will generally pass (it is boolean true).

Recommendation is to use the UtAssert_Abort() function instead.

Add transaction watchpoint concept for transaction complete notification

This issue was imported from the GSFC issue tracking system

Imported from: [GSFCCFS-1211] transaction watchpoint
Originally submitted by: Seeger, Steven D. (GSFC-582.0)[Embedded Flight Systems, Inc] on Thu Aug 6 11:02:12 2020

Original Description:
It might be useful for operators to have a concept of transaction watchpoints. This could be implemented with the spare 16-bit per-channel register (or the spare 8-bit per-channel) coupled with a ground command specifying a bit in that register to be set when a transaction is complete. The benefit here is a proc could request a watchpoint and wait for the bit to be set when the file is done. This is more useful than a file counter because polling directories being active could skew the file counter.

Some CF tests appear to use the buffer from the previous test

This issue was imported from the GSFC issue tracking system

Imported from: [GSFCCFS-1781] Some CF tests appear to use the buffer from the previous test
Originally submitted by: Hickey, Joseph P. (GSFC-582.0)[VANTAGE SYSTEMS INC] on Tue Nov 16 17:04:15 2021

Original Description:
The following CF test functions do not instantiate their own message buffer, but rather use the value contained in the global CF_AppData.engine.in.msg:

1. Test_CF_CFDP_R_ProcessFd_NoCrc_cached_pos_NotEqTo_offset_And_fret_NotEqTo_offset_SendEventSetAndCountErrorReturn_neg1
2. Test_CF_CFDP_R_ProcessFd_NoCrc_fret_NotEqTo_bytes_received_Value_SendEventSetAndCountErrorReturn_neg1
3. Test_CF_CFDP_R_ProcessFd_NoCrc_cached_pos_Gets_bytes_received_Plus_offset_And_data_bytes_IncreasesBy_bytes_received_Return_0
4. Test_CF_CFDP_R_ProcessFd_NoCrc_cached_pos_NotEqTo_offset_But_fret_IsEqTo_offset_cached_pos_Gets_bytes_received_Plus_offset_And_data_bytes_IncreasesBy_bytes_received_Return_0

This pointer likely points at a stack location from the _previous_ test function (whatever last set the msg pointer). Therefore the memory it points to at the time these tests execute is no longer valid. If anything writes to this memory, it will corrupt the local stack and potentially segfault/crash the test.

Failures in sending files from cFS and ground software

Hi,

About 3 months ago, I asked the same question, but I didn't get any responses.
So I am asking the question again.

I have been using CF app's version 6.8.
I understand that NASA is going to release an updated version of CF app with major changes sometime next year, but I need to resolve the following issue soon.

Whenever I tried to send a file (size of 200 bytes) from cFS (version 6.8) to ground software (cosmos or yamcs), I can send only 3 times. Here's example with sending a small size file (it has 27 bytes) 4 times in a row.
At the 4th try, cFS prints out "started", but no "success" message after that.

EVS Port1 43/1/CF 103: Outgoing trans started 0.24_1,src /cf/test.txt
EVS Port1 43/1/CF 21: Outgoing trans success 0.24_1,src /cf/test.txt

EVS Port1 43/1/CF 103: Outgoing trans started 0.24_2,src /cf/test.txt
EVS Port1 43/1/CF 21: Outgoing trans success 0.24_2,src /cf/test.txt

EVS Port1 43/1/CF 103: Outgoing trans started 0.24_3,src /cf/test.txt
EVS Port1 43/1/CF 21: Outgoing trans success 0.24_3,src /cf/test.txt

EVS Port1 43/1/CF 103: Outgoing trans started 0.24_4,src /cf/test.txt
EVS Port1 43/1/CF 80: CF:Playback File Cmd Error, File is Already Pending or Active:/cf/test.txt

• If you try to send a file bigger than 500 bytes or so, this problem can occur at the second or 3rd try.
• When I try to send a file with more than 1600 bytes to ground software, it doesn't work at all.
• I experimented with a file that has 1600 bytes. cFS was able to send the file to ground software, but with a file with 1601 bytes, it couldn't. Also after sending the 1600 byte file, cFS can't send another file regardless of the file size.

Sending a file from ground software to cFS works fine, but cFS can't finish sending a file from cFS to ground software after that.
Overall, it is hard to predict what cFS would do when it comes to sending a file to ground software.

Did anyone have the same issue as me?

Thanks,
Harry Kim

Length error for little-endian

CF/fsw/src/cf_app.c

Line 1023 in cbda7b9

CF_AppData.RawPduInputBuf.length = PduHdrPtr->PDataLen + PduHdrBytes;

The line above works for big-endian systems, however not consistent with the line below for little-endian systems. pdu->length calculated and checked below line w.r.t. big-endian, but the same calculation and check done above w.r.t. endianness of system. Running the app in a little-endian system results in length error for check if(CF_AppData.RawPduInputBuf.length > CF_INCOMING_PDU_BUF_SIZE) after the line above.

CF/fsw/src/PRI/pdu.c

Line 420 in cbda7b9

data_field_length = (pdu->content[1] * 256) + pdu->content[2];

SOLUTION:

cf_app.c#L1023:

Previous Version:

CF_AppData.RawPduInputBuf.length = PduHdrPtr->PDataLen + PduHdrBytes; 

Next Version:

CF_AppData.RawPduInputBuf.length = MAKE_BIG16(PduHdrPtr->PDataLen) + PduHdrBytes;

CF command tests need to use union when instantiating objects of type CFE_SB_Buffer_t

This issue was imported from the GSFC issue tracking system

Imported from: [GSFCCFS-1785] CF command tests need to use union when instantiating objects of type CFE_SB_Buffer_t
Originally submitted by: Hickey, Joseph P. (GSFC-582.0)[VANTAGE SYSTEMS INC] on Tue Nov 16 17:29:44 2021

Original Description:
This is similar in nature to previous issue described in #44 but in the "cf_cmd_tests.c" file.

In this instance, command buffers are instantiated on the stack, but then cast to CFE_SB_Buffer_t*. The stack variables are not correctly aligned for this cast to be valid, and many compilers will (correctly) trigger a warning/error about this.

Solution is to use a union to ensure alignment, where code like:

    cf_cmd_unionargs_t dummy_msg;
    CFE_SB_Buffer_t* arg_msg = (CFE_SB_Buffer_t*)&dummy_msg;

Needs to become:

    union
    {
        cf_cmd_unionargs_t msg;
        CFE_SB_Buffer_t buf;
    } dummy;

such that &dummy.buf can serve as the pointer to pass to a function accepting a CFE_SB_Buffer_t*.

CF_CFDP_PlaybackDir_ declared with fixed-length array parameters

This issue was imported from the GSFC issue tracking system

Imported from: [GSFCCFS-1779] CF_CFDP_PlaybackDir_ declared with fixed-length array parameters
Originally submitted by: Hickey, Joseph P. (GSFC-582.0)[VANTAGE SYSTEMS INC] on Tue Nov 16 16:49:50 2021

Original Description:
Typically, functions which accept a zero-terminated string are passed a pointer to the string.

However, CF_CFDP_PlaybackDir_ is declared as accepting a fixed-length array buffer, where the length is specified as CF_FILENAME_MAX_LEN. For example:

CF_CFDP_PlaybackDir_(playback_t *p, const char src_filename[CF_FILENAME_MAX_LEN], const char dst_filename[CF_FILENAME_MAX_LEN], cfdp_class_t cfdp_class, uint8 keep, uint8 chan, uint8 priority, cf_entity_id_t dest_id)

The problem with declaring it this way is that it implies the full length is required, and should _only_ be called with a fixed-length array of that size. Calling it with a shorter buffer, such as CF_FILENAME_MAX_PATH, is invalid, even if the buffer is null terminated.

In CF, this function is invoked from CF_CFDP_ProcessPollingDirectories, with a buffer of size CF_FILENAME_MAX_PATH - which is not the same size. This results in an error/warning on compilers that check for this type of thing.

Recommend to change this to a const char *

CF method CF_HeaderSize returns an int, but it should be a size_t

This issue was imported from the GSFC issue tracking system

Imported from: [GSFCCFS-1738] CF method CF_HeaderSize returns an int, but it should be a size_t
Originally submitted by: Gibson, Alan S. (GSFC-5870) on Mon Oct 4 08:05:04 2021

Original Description:
The CF_HeaderSize function determines the header size of the given pdu_header_t and returns this value. However, it is using an int type as the return, but it should never be able to be negative and it returns a sizeof value. There is no reason for it to be an int return type; initial unit tests did show that it can technically return values that are far too large and negative values that will crash the app.

It should at a minimum be changed to a size_t, but recommend something more like a uint8 as the practicality of it returning something larger than 255 is not likely to happen.

CF C99 compliance and use of packed structures

This issue was imported from the GSFC issue tracking system

Imported from: [GSFCCFS-1777] CF C99 compliance and use of packed structures
Originally submitted by: Hickey, Joseph P. (GSFC-582.0)[VANTAGE SYSTEMS INC] on Tue Nov 16 15:53:48 2021

Original Description:
CF currently uses the 'CF_PACK` attribute on many of its structure types, which translates to the gcc "__attribute__((packed))" extension, without any provision for other compilers.

This is a nonstandard/compiler-specific extension feature and makes the code NOT c99-compliant. Attempting to build this code with non-gcc compiler will likely fail as a result.

Application source code should be limited to standard C99 features, and should not rely on vendor-specific extensions.

Doesn't work by default with OSAL 4.2.1.0 official release

I'm using the rc-2.2.2 branch with cfe 6.6 and OSAL 4.2.1.0. It doesn't build without the following changes:

1. fsw/src/cf_callbacks.c:1140 - st_size should be FileSize
2. fsw/src/cf_playback.c - all references to os_dirent_t's d_name entry should be changed to FileName.

Unused event ID

This issue was imported from the GSFC issue tracking system

Imported from: [GSFCCFS-1703] Unused event ID
Originally submitted by: Timmons, Elizabeth J. (GSFC-5820) on Tue Aug 10 20:51:47 2021

Original Description:
The event ID CF_EID_ERR_PDU_BAD_RX_MSG_SIZE appears to be unused.

CF use of STATIC_CAST without NULL checks

This issue was imported from the GSFC issue tracking system

Imported from: [GSFCCFS-1773] CF use of STATIC_CAST without NULL checks
Originally submitted by: Hickey, Joseph P. (GSFC-582.0)[VANTAGE SYSTEMS INC] on Tue Nov 16 14:05:37 2021

Original Description:
This issue was originally reported by IV&V, creating Jira issue to track its disposition and resolution.

There are 7 instances in the CFS CF App source code in which the PDU hedaer pointer 'ph' can be a NULL value from a prior function call, and then it is passed into the function STATIC_CAST() where it is eventually dereferenced without any checks.

These calls to STATIC_CAST() pass 'ph' to CF_HeaderSize() which dereferences the pointer.

cf_cfdp.c line 476
cf_cfdp.c line 616
cf_cfdp.c line 654
cf_cfdp.c line 695
cf_cfdp_r.c line 491
cf_cfdp_s.c line 125
cf_cfdp.c line 1277

CF Assertions should not be compiled in when using "release" buildtype

This issue was imported from the GSFC issue tracking system

Imported from: [GSFCCFS-1787] CF Assertions should not be compiled in when using "release" buildtype
Originally submitted by: Hickey, Joseph P. (GSFC-582.0)[VANTAGE SYSTEMS INC] on Tue Nov 16 18:42:37 2021

Original Description:
See also #23 regarding the use of "unlikely" in this macro.

When building with BUILDTYPE=release, the NDEBUG macro will be set. Typically this turns off assertions in the code, but in CF this is not the case, it redefines CF_Assert to a local handler instead.

Typically code will disable/compile-out assertion statements when in release mode, as they should never be triggered, so they just waste cycles.

Investigate whether TX and RX PDU sizes need to be different

This issue was imported from the GSFC issue tracking system

Imported from: [GSFCCFS-1196] Investigate whether TX and RX PDU sizes need to be different
Originally submitted by: Timmons, Elizabeth J. (GSFC-5820) on Mon Jul 20 13:37:06 2020

Original Description:
Need to investigate uses cases where TX and RX PDU sizes need to be different. Need to investigate the impact of making a change to allow that.

CF-3.0 having a problem building unit tests with unlikely((x))

This issue was imported from the GSFC issue tracking system

Imported from: [GSFCCFS-1217] CF-3.0 having a problem building unit tests with unlikely((x))
Originally submitted by: Gibson, Alan S. (GSFC-5870) on Mon Aug 10 15:05:16 2020

Original Description:
When building with
make ENABLE_UNIT_TESTS=TRUE SIMULATION=native
and
NDEBUG defined
I get a undefined reference unlikely error.
Determine if we need to keep unlikely and if so, what needs done to get it to build with the unit tests.

Buffer overflow in machine_list.c

static boolean m_is_this_slot_in_use [MAX_CONCURRENT_TRANSACTIONS];

...

  i = 0;
  while (m_is_this_slot_in_use[i] && (i < MAX_CONCURRENT_TRANSACTIONS))
    i ++;

Assume MAX_CONCURRENT_TRANSACTIONS = 2

On the first iteration of the loop, m_is_this_slot_in_use[i] will access array index 0, on the second pass array index 1, and on the third pass the out-of-bounds index 2, causing a buffer overflow.

The following simple fix will short-circuit the while loop, preventing the buffer overflow:

while ((i < MAX_CONCURRENT_TRANSACTIONS) && m_is_this_slot_in_use[i])

File size limitation when sending files from cFS and ground software

Whenever I tried to send a file (size of 200 bytes) from cFS (version 6.8) to ground software (cosmos or yamcs), I can send only 3 times. At the 4th try, cFS prints out "started", but no "success" message after that.

EVS Port1 43/1/CF 103: Outgoing trans started 0.24_2,src /cf/cf_test.txt
EVS Port1 43/1/CF 21: Outgoing trans success 0.24_2,src /cf/cf_test.txt
EVS Port1 43/1/CF 103: Outgoing trans started 0.24_1,src /cf/cf_test.txt
EVS Port1 43/1/CF 80: CF:Playback File Cmd Error, File is Already Pending or Active:/cf/cf_test.txt

If you try to send a file bigger than 500 bytes or so, this problem can occur at the second or 3rd try.

Also when I try to send a file with more than 1600 bytes to ground software, it doesn't work at all.
I experimented with a file that has 1600 bytes and cFS was able to finish the job, but with a file with 1601 bytes, it couldn't finish the job. Also after sending the 1600 byte file, cFS can't send another file regardless of the file size.

Sending a file from ground software to cFS works fine, but cFS can't finish sending a file from cFS to ground software after that.
Overall, it is hard to predict what cFS would do when it comes to sending a file to ground software.

Did anyone have the same issue as me?

CF test must not instantiate global variables in a header file

This issue was imported from the GSFC issue tracking system

Imported from: [GSFCCFS-1786] CF test must not instantiate global variables in a header file
Originally submitted by: Hickey, Joseph P. (GSFC-582.0)[VANTAGE SYSTEMS INC] on Tue Nov 16 17:37:09 2021

Original Description:
The 'cf_test_utils.h' instantiates (rather than simply declaring) a number of variables:

Lines 45-46
int32 result;
uint16 EventID;

Line 87:
CFE_MSG_SetMsgTime_context_t context_CFE_MSG_SetMsgTime;

Line 93:
CFE_MSG_GetMsgId_context_t context_CFE_MSG_GetMsgId;

Line 100
CFE_EVS_SendEvent_context_t context_CFE_EVS_SendEvent;

Line 106:
CFE_MSG_GetSize_context_t context_CFE_MSG_GetSize;

Line 382:
type_of_context_CF_CList_Traverse_t type_of_context_CF_CList_Traverse;

Preferably, tests should be designed to not need global state (pass in the buffers as needed).

If this is not possible, then for cases where these are actually accessed from multiple source files, the header should only declare the variable as "extern" and instantiate in the most appropriate .c file.

Note for the case of CFE_MSG_GetSize -- this does not appear to be accessed from more than one C file so it likely does not even need to be "extern" - it can be just scoped to the one file that uses it.

The tests fail to link due to this problem, because of the multiple definitions of these variable names.

CF - Incorrect cast in test functions

This issue was imported from the GSFC issue tracking system

Imported from: [GSFCCFS-1782] CF - Incorrect cast in test functions
Originally submitted by: Hickey, Joseph P. (GSFC-582.0)[VANTAGE SYSTEMS INC] on Tue Nov 16 17:11:06 2021

Original Description:
Some CF test funtions incorrectly cast the pdu_header_t as a different type of header, for example inside 'Test_CF_CFDP_RecvIdle_CheckOf_PDU_HDR_FLAGS_TYPE_Returns_false_But_fdh_directive_code_IsNot_PDU_METADATA_SendEventAnd_Increment_recv_error' it does this:

    ((pdu_file_directive_header_t*)&dummy_msg.pdu_r_msg.ph)->directive_code = Any_file_directive_t_Except(PDU_METADATA);

The problem here is that the pdu_file_directive_header_t should _follow_ the standard pdu_header_t (ph), as it is an extension of this header, it does not replace this header. As a result this is not writing the value in the location expected.

CF method CF_CFDP_SendMd cannot get negative to return from CF_CFDP_CopyDataToLv

This issue was imported from the GSFC issue tracking system

Imported from: [GSFCCFS-1737] CF method CF_CFDP_SendMd cannot get negative to return from CF_CFDP_CopyDataToLv
Originally submitted by: Gibson, Alan S. (GSFC-5870) on Wed Sep 29 16:44:26 2021

Original Description:
Two times a ret value from CF_CFDP_CopyDataToLv is checked to see if it is negative (<0) and neither call appears to every be able to return a negative value.

Even the comments in the code allude to this fact:
/* should not happen, since filename lengths are checked above */

(link removed)

(link removed)

If this cannot happen then there is no need for the code to check for it? Unsure how to proceed with this, but it cannot be tested with unit tests.

sending a text file from cosmos to cfs has failed due to size mismatch.

Hello,

To test if I can send a simple text file from cosmos to cfs,
I used a text file that contains one sentence "This is a test file."
I had the following error while testing with cf v6.8 although I didn't have this error with v6.7.

EVS Port1 43/1/CF 18: cfdp_engine: file size mismatch -- 8675833937941 / 4152025208 (eof/received)).

This error was from apps/cf/fsw/src/PRI/aaa.c line 388

boolean aaa__is_file_size_valid (MACHINE m)
{
TRANS_STATUS mp = &(m->publik); / useful shorthand /
/------------------------------------------------------------/
if (mp->received_file_size > m->eof.file_size)
{
e_msg__ ("cfdp_engine: file size mismatch -- %Lu / %lu "
"(eof/received))\n",
m->eof.file_size, mp->received_file_size);
return (NO);
}
return (YES);
}

Any idea why this error occurred?
Thanks in advance.