Skip to content

Commit

Permalink
Fix #441, cf delete files during error when tx
Browse files Browse the repository at this point in the history
Fix #441, Update to handle error during tx. For polling, the file gets moved or deleted to pervent infinite loop. For others, the file does not get deleted. Update Unit Test and documentation.
  • Loading branch information
Anh D Van committed Aug 1, 2024
1 parent 86e5136 commit b357f17
Show file tree
Hide file tree
Showing 7 changed files with 280 additions and 37 deletions.
1 change: 1 addition & 0 deletions config/default_cf_tblstruct.h
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ typedef struct CF_ConfigTable
uint16 outgoing_file_chunk_size; /**< \brief maximum size of outgoing file data chunk in a PDU.
* Limited by CF_MAX_PDU_SIZE minus the PDU header(s) */
char tmp_dir[CF_FILENAME_MAX_PATH]; /**< \brief directory to put temp files */
char fail_dir[CF_FILENAME_MAX_PATH]; /**< \brief fail directory */
} CF_ConfigTable_t;

#endif
31 changes: 20 additions & 11 deletions docs/dox_src/cfs_cf.dox
Original file line number Diff line number Diff line change
Expand Up @@ -398,16 +398,24 @@
When files are found in a polling directory that is enabled, the files are
automatically queued by CF for output. The polling rate is configurable and
each channel has a configurable number of polling directories. Each polling
directory has an enable, a class setting, a priority setting, a preserve
setting and a destination directory. When enabled, CF will periodically check
the polling directories for files. When a file is found, CF will place the
file information on the pending queue if it is closed and not already on the
queue and not currently active. All polling directories are checked at the
same frequency which is defined by the table parameter
NumWakeupsPerPollDirChk. Setting this parameter to one will cause CF to check
the polling directories at the fastest possible rate, every time it receives a
'wake-up' command. Checking polling directories is a processor-intensive
effort, it is best to keep the polling rate as low as possible.
directory has an enable, a class setting, a priority setting, a destination
directory. When enabled, CF will periodically check the polling directories
for files. When a file is found, CF will place the file information on the
pending queue if it is closed and not already on the queue and not currently
active. All polling directories are checked at the same frequency which is
defined by the table parameter NumWakeupsPerPollDirChk. Setting this parameter
to one will cause CF to check the polling directories at the fastest possible
rate, every time it receives a 'wake-up' command. Checking polling directories
is a processor-intensive effort, it is best to keep the polling rate as low as
possible.

<H2> Failed Transmit and directory use </H2>

Currently, the fail directory is used only to store class 2 TX files that failed
during transmission from a polling directory. When a file fails during transmission
from a polling directory, it is deleted from the polling directory to prevent an
infinite loop and moved to the fail directory. However, if a file fails during a
class 2 TX outside of a polling directory, the file will not be deleted.

<H2> Efficiency </H2>

Expand Down Expand Up @@ -645,7 +653,8 @@ CF_UnionArgs_Payload_t;
and an event will be generated.

If the command is not successful, the command error counter will increment and
an error event will be generated indicating the reason for failure.
an error event will be generated indicating the reason for failure. File will
not be deleted if the 'transmit' failed (unless in an polling directories).

\verbatim
typedef struct CF_TxFileCmd
Expand Down
126 changes: 100 additions & 26 deletions fsw/src/cf_cfdp.c
Original file line number Diff line number Diff line change
Expand Up @@ -1588,9 +1588,7 @@ void CF_CFDP_CycleEngine(void)
*-----------------------------------------------------------------*/
void CF_CFDP_ResetTransaction(CF_Transaction_t *txn, int keep_history)
{
char * filename;
char destination[OS_MAX_PATH_LEN];
osal_status_t status = OS_ERROR;

CF_Channel_t *chan = &CF_AppData.engine.channels[txn->chan_num];
CF_Assert(txn->chan_num < CF_NUM_CHANNELS);

Expand All @@ -1608,31 +1606,10 @@ void CF_CFDP_ResetTransaction(CF_Transaction_t *txn, int keep_history)
if (OS_ObjectIdDefined(txn->fd))
{
CF_WrappedClose(txn->fd);

if (!txn->keep)
{
if (CF_CFDP_IsSender(txn))
{
/* If move directory is defined attempt move */
if (CF_AppData.config_table->chan[txn->chan_num].move_dir[0] != 0)
{
filename = strrchr(txn->history->fnames.src_filename, '/');
if (filename != NULL)
{
snprintf(destination, sizeof(destination), "%s%s",
CF_AppData.config_table->chan[txn->chan_num].move_dir, filename);
status = OS_mv(txn->history->fnames.src_filename, destination);
}
}

if (status != OS_SUCCESS)
{
OS_remove(txn->history->fnames.src_filename);
}
}
else
{
OS_remove(txn->history->fnames.dst_filename);
}
CF_CFDP_HandleNotKeepFile(txn);
}
}

Expand Down Expand Up @@ -1829,3 +1806,100 @@ void CF_CFDP_DisableEngine(void)
CFE_SB_DeletePipe(chan->pipe);
}
}

/*----------------------------------------------------------------
*
* Application-scope internal function
* See description in cf_cfdp.h for argument/return detail
*
*-----------------------------------------------------------------*/
bool CF_CFDP_IsPollingDir(const char * src_file, uint8 chan_num)

Check notice

Code scanning / CodeQL

Long function without assertion Note

All functions of more than 10 lines should have at least one assertion.
{
bool return_code = false;
char src_dir[CF_FILENAME_MAX_LEN] = "\0";
CF_ChannelConfig_t *cc;
CF_PollDir_t * pd;
int i;

char * last_slash = strrchr(src_file, '/');
if(last_slash != NULL)
{
strncpy(src_dir, src_file, last_slash - src_file);
}

cc = &CF_AppData.config_table->chan[chan_num];
for (i = 0; i < CF_MAX_POLLING_DIR_PER_CHAN; ++i)
{
pd = &cc->polldir[i];
if(strcmp(src_dir, pd->src_dir) == 0)
{
return_code = true;
break;
}
}

return return_code;
}

/*----------------------------------------------------------------
*
* Application-scope internal function
* See description in cf_cfdp.h for argument/return detail
*
*-----------------------------------------------------------------*/
void CF_CFDP_HandleNotKeepFile(CF_Transaction_t *txn)

Check notice

Code scanning / CodeQL

Long function without assertion Note

All functions of more than 10 lines should have at least one assertion.
{
/* Sender */
if (CF_CFDP_IsSender(txn))

Check warning

Code scanning / CodeQL

Side effect in a Boolean expression Warning

This Boolean expression is not side-effect free.
{
if(!CF_TxnStatus_IsError(txn->history->txn_stat))
{
/* If move directory is defined attempt move */
CF_CFDP_MoveFile(txn->history->fnames.src_filename, CF_AppData.config_table->chan[txn->chan_num].move_dir);
}
else
{
/* file inside an polling directory */
if(CF_CFDP_IsPollingDir(txn->history->fnames.src_filename, txn->chan_num))
{
/* If fail directory is defined attempt move */
CF_CFDP_MoveFile(txn->history->fnames.src_filename, CF_AppData.config_table->fail_dir);
}
}
}
/* Not Sender */
else
{
OS_remove(txn->history->fnames.dst_filename);
}
}


/*----------------------------------------------------------------
*
* Application-scope internal function
* See description in cf_cfdp.h for argument/return detail
*
*-----------------------------------------------------------------*/
void CF_CFDP_MoveFile(const char *src, const char *dest_dir)

Check notice

Code scanning / CodeQL

Long function without assertion Note

All functions of more than 10 lines should have at least one assertion.
{
osal_status_t status = OS_ERROR;
char * filename;
char destination[OS_MAX_PATH_LEN];

if(dest_dir[0] != 0)
{
filename = strrchr(src, '/');
if (filename != NULL)
{
snprintf(destination, sizeof(destination), "%s%s",
dest_dir, filename);
status = OS_mv(src, destination);
}
}

if (status != OS_SUCCESS)
{
OS_remove(src);
}
}
30 changes: 30 additions & 0 deletions fsw/src/cf_cfdp.h
Original file line number Diff line number Diff line change
Expand Up @@ -703,4 +703,34 @@ void CF_CFDP_ProcessPollingDirectories(CF_Channel_t *chan);
*/
CF_CListTraverse_Status_t CF_CFDP_DoTick(CF_CListNode_t *node, void *context);

/************************************************************************/
/** @brief Check if source file came from polling directory
*
*
* @par Assumptions, External Events, and Notes:
*
* @retval true/false
*/
bool CF_CFDP_IsPollingDir(const char * src_file, uint8 chan_num);

/************************************************************************/
/** @brief Remove/Move file after transaction
*
* This helper is used to handle "not keep" file option after a transaction.
*
* @par Assumptions, External Events, and Notes:
*
*/
void CF_CFDP_HandleNotKeepFile(CF_Transaction_t *txn);

/************************************************************************/
/** @brief Move File
*
* This helper is used to move a file.
*
* @par Assumptions, External Events, and Notes:
*
*/
void CF_CFDP_MoveFile(const char *src, const char *dest_dir);

#endif /* !CF_CFDP_H */
1 change: 1 addition & 0 deletions fsw/tables/cf_def_config.c
Original file line number Diff line number Diff line change
Expand Up @@ -80,5 +80,6 @@ CF_ConfigTable_t CF_config_table = {
.move_dir = ""}},
480, /* outgoing_file_chunk_size */
"/cf/tmp", /* temporary file directory */
"/cf/fail", /* Stores failed tx file for "polling directory" */
};
CFE_TBL_FILEDEF(CF_config_table, CF.config_table, CF config table, cf_def_config.tbl)
86 changes: 86 additions & 0 deletions unit-test/cf_cfdp_tests.c
Original file line number Diff line number Diff line change
Expand Up @@ -1357,6 +1357,7 @@ void Test_CF_CFDP_ResetTransaction(void)
CF_History_t * history;
CF_Channel_t * chan;
CF_Playback_t pb;
CF_ConfigTable_t *config;

memset(&pb, 0, sizeof(pb));

Expand Down Expand Up @@ -1443,6 +1444,91 @@ void Test_CF_CFDP_ResetTransaction(void)
UtAssert_UINT32_EQ(pb.num_ts, 9);
UtAssert_UINT32_EQ(chan->num_cmd_tx, 7);
UtAssert_STUB_COUNT(CF_FreeTransaction, 1);

/*
* File is in Polling Directory, Not Keep, and is Error
* Move to fail directory successful
*/
UT_ResetState(UT_KEY(CF_FreeTransaction));
UT_ResetState(UT_KEY(OS_remove));
UT_ResetState(UT_KEY(OS_mv));
UT_CFDP_SetupBasicTestState(UT_CF_Setup_TX, NULL, &chan, &history, &txn, &config);
UT_SetDefaultReturnValue(UT_KEY(CF_TxnStatus_IsError), true);\
UT_SetDefaultReturnValue(UT_KEY(OS_mv), OS_SUCCESS);
txn->fd = OS_ObjectIdFromInteger(1);
txn->keep = 0;
txn->state = CF_TxnState_S2;
strcpy(history->fnames.src_filename, "/ram/poll1/test1");
strcpy(config->chan[0].polldir[0].src_dir, "/ram/poll1");
strcpy(config->fail_dir, "/ram/fail");

UtAssert_VOIDCALL(CF_CFDP_ResetTransaction(txn, 0));
UtAssert_STUB_COUNT(CF_FreeTransaction, 1);
UtAssert_STUB_COUNT(OS_mv, 1);
UtAssert_STUB_COUNT(OS_remove, 0);

/*
* File is in Polling Directory, Not Keep, and is Error
* Move to fail directory not successful
*/
UT_ResetState(UT_KEY(CF_FreeTransaction));
UT_ResetState(UT_KEY(OS_remove));
UT_ResetState(UT_KEY(OS_mv));
UT_CFDP_SetupBasicTestState(UT_CF_Setup_TX, NULL, &chan, &history, &txn, &config);
UT_SetDefaultReturnValue(UT_KEY(CF_TxnStatus_IsError), true);
UT_SetDefaultReturnValue(UT_KEY(OS_mv), OS_ERROR);
txn->fd = OS_ObjectIdFromInteger(1);
txn->keep = 0;
txn->state = CF_TxnState_S2;
strcpy(history->fnames.src_filename, "/ram/poll1/test1");
strcpy(config->chan[0].polldir[0].src_dir, "/ram/poll1");
strcpy(config->fail_dir, "/ram/fail");

UtAssert_VOIDCALL(CF_CFDP_ResetTransaction(txn, 0));
UtAssert_STUB_COUNT(CF_FreeTransaction, 1);
UtAssert_STUB_COUNT(OS_mv, 1);
UtAssert_STUB_COUNT(OS_remove, 1);

/*
* Source file outside polling directory, Not Keep, is Error
*/
UT_ResetState(UT_KEY(CF_FreeTransaction));
UT_ResetState(UT_KEY(OS_remove));
UT_ResetState(UT_KEY(OS_mv));
UT_CFDP_SetupBasicTestState(UT_CF_Setup_TX, NULL, &chan, &history, &txn, &config);
UT_SetDefaultReturnValue(UT_KEY(CF_TxnStatus_IsError), true);
txn->fd = OS_ObjectIdFromInteger(1);
txn->keep = 0;
txn->state = CF_TxnState_S2;
strcpy(history->fnames.src_filename, "/ram/test.txt");
strcpy(config->chan[0].polldir[0].src_dir, "/ram/poll1");
strcpy(config->fail_dir, "/ram/fail");

UtAssert_VOIDCALL(CF_CFDP_ResetTransaction(txn, 0));
UtAssert_STUB_COUNT(CF_FreeTransaction, 1);
UtAssert_STUB_COUNT(OS_mv, 0);
UtAssert_STUB_COUNT(OS_remove, 0);

/*
* Source File is empty string. Not Keep, is Error
*/
UT_ResetState(UT_KEY(CF_FreeTransaction));
UT_ResetState(UT_KEY(OS_remove));
UT_ResetState(UT_KEY(OS_mv));
UT_CFDP_SetupBasicTestState(UT_CF_Setup_TX, NULL, &chan, &history, &txn, &config);
UT_SetDefaultReturnValue(UT_KEY(CF_TxnStatus_IsError), true);
txn->fd = OS_ObjectIdFromInteger(1);
txn->keep = 0;
txn->state = CF_TxnState_S2;
strcpy(history->fnames.src_filename, "");
strcpy(config->chan[0].polldir[0].src_dir, "/ram/poll1");
strcpy(config->fail_dir, "/ram/fail");

UtAssert_VOIDCALL(CF_CFDP_ResetTransaction(txn, 0));
UtAssert_STUB_COUNT(CF_FreeTransaction, 1);
UtAssert_STUB_COUNT(OS_mv, 0);
UtAssert_STUB_COUNT(OS_remove, 1);

}

void Test_CF_CFDP_SetTxnStatus(void)
Expand Down
42 changes: 42 additions & 0 deletions unit-test/stubs/cf_cfdp_stubs.c
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,18 @@ void CF_CFDP_EncodeStart(CF_EncoderState_t *penc, void *msgbuf, CF_Logical_PduBu
UT_GenStub_Execute(CF_CFDP_EncodeStart, Basic, NULL);
}

/*
* ----------------------------------------------------
* Generated stub function for CF_CFDP_HandleNotKeepFile()
* ----------------------------------------------------
*/
void CF_CFDP_HandleNotKeepFile(CF_Transaction_t *txn)
{
UT_GenStub_AddParam(CF_CFDP_HandleNotKeepFile, CF_Transaction_t *, txn);

UT_GenStub_Execute(CF_CFDP_HandleNotKeepFile, Basic, NULL);
}

/*
* ----------------------------------------------------
* Generated stub function for CF_CFDP_InitEngine()
Expand Down Expand Up @@ -273,6 +285,36 @@ void CF_CFDP_InitTxnTxFile(CF_Transaction_t *txn, CF_CFDP_Class_t cfdp_class, ui
UT_GenStub_Execute(CF_CFDP_InitTxnTxFile, Basic, NULL);
}

/*
* ----------------------------------------------------
* Generated stub function for CF_CFDP_IsPollingDir()
* ----------------------------------------------------
*/
bool CF_CFDP_IsPollingDir(const char *src_file, uint8 chan_num)
{
UT_GenStub_SetupReturnBuffer(CF_CFDP_IsPollingDir, bool);

UT_GenStub_AddParam(CF_CFDP_IsPollingDir, const char *, src_file);
UT_GenStub_AddParam(CF_CFDP_IsPollingDir, uint8, chan_num);

UT_GenStub_Execute(CF_CFDP_IsPollingDir, Basic, NULL);

return UT_GenStub_GetReturnValue(CF_CFDP_IsPollingDir, bool);
}

/*
* ----------------------------------------------------
* Generated stub function for CF_CFDP_MoveFile()
* ----------------------------------------------------
*/
void CF_CFDP_MoveFile(const char *src, const char *dest_dir)
{
UT_GenStub_AddParam(CF_CFDP_MoveFile, const char *, src);
UT_GenStub_AddParam(CF_CFDP_MoveFile, const char *, dest_dir);

UT_GenStub_Execute(CF_CFDP_MoveFile, Basic, NULL);
}

/*
* ----------------------------------------------------
* Generated stub function for CF_CFDP_PlaybackDir()
Expand Down

0 comments on commit b357f17

Please sign in to comment.