Merge pull request nasa#459 from avan989/Fix-441-cf-delete-file-trans…

…mit-error

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

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