Fix nasa#441, cf delete files during error when tx

Fix nasa#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.

config/default_cf_tblstruct.h

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

docs/dox_src/cfs_cf.dox

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

fsw/src/cf_cfdp.c

@@ -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);
 
@@ -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);
         }
     }
 
@@ -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)
+{
+    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)
+{
+    /* Sender */
+    if (CF_CFDP_IsSender(txn))
+    {
+        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)
+{
+    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);
+    }
+}

fsw/src/cf_cfdp.h

@@ -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 */

fsw/tables/cf_def_config.c

@@ -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)

unit-test/cf_cfdp_tests.c

@@ -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));
 
@@ -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)

unit-test/stubs/cf_cfdp_stubs.c

@@ -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()
@@ -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()