More Zmodem bug fixes: Too many pkttypes; Use wrong packet buffer

4 files changed, 51 insertions, 28 deletions

diff --git a/apps/system/zmodem/zm.h b/apps/system/zmodem/zm.h
index 10f4640c5..2dbafc9e4 100644
--- a/apps/system/zmodem/zm.h
+++ b/apps/system/zmodem/zm.h
@@ -369,7 +369,6 @@ struct zmr_state_s
   uint8_t f2;                /* Transfer flag F2 */
   uint8_t f3;                /* Transfer flag F3 */
 #endif
-  uint8_t pkttype;           /* Type of packet */
   uint8_t ntimeouts;         /* Number of timeouts */
   uint32_t crc;              /* Remove file CRC */
   FAR char *filename;        /* Local filename */
diff --git a/apps/system/zmodem/zm_receive.c b/apps/system/zmodem/zm_receive.c
index 537da2c5f..c26d15e02 100644
--- a/apps/system/zmodem/zm_receive.c
+++ b/apps/system/zmodem/zm_receive.c
@@ -319,6 +319,7 @@ static int zmr_zrinit(FAR struct zm_state_s *pzm)
 
   zmdbg("ZMR_STATE %d:->%d Send ZRINIT\n", pzm->state, ZMR_START);
   pzm->state   = ZMR_START;
+  pzm->flags  &= ~ZM_FLAG_OO;   /* In case we get here from ZMR_FINISH */
 
   /* Send ZRINIT */
 
@@ -409,7 +410,7 @@ static int zmr_zsrintdata(FAR struct zm_state_s *pzm)
   FAR struct zmr_state_s *pzmr = (FAR struct zmr_state_s *)pzm;
   uint8_t by[4];
 
-  zmdbg("PSTATE %d:%d->%d.%d. Received the rest of the ZSINIT packet\n",
+  zmdbg("PSTATE %d:%d->%d:%d. Received the rest of the ZSINIT packet\n",
         pzm->pstate, pzm->psubstate, PSTATE_IDLE, PIDLE_ZPAD);
 
   pzm->pstate    = PSTATE_IDLE;
@@ -513,6 +514,7 @@ static int zmr_zfile(FAR struct zm_state_s *pzm)
   zmdbg("ZMR_STATE %d\n", pzm->state);
 
   pzm->nerrors = 0;
+  pzm->flags  &= ~ZM_FLAG_OO;   /* In case we get here from ZMR_FINISH */
 
   /* Cache flags (skipping of the initial header type byte) */
 
@@ -609,7 +611,7 @@ static int zmr_filename(FAR struct zm_state_s *pzm)
   int filetype;
   int ret;
 
-  zmdbg("PSTATE %d:%d->%d.%d\n",
+  zmdbg("PSTATE %d:%d->%d:%d\n",
         pzm->pstate, pzm->psubstate, PSTATE_IDLE, PIDLE_ZPAD);
   zmdbg("ZMR_STATE %d\n", pzm->state);
 
@@ -734,7 +736,7 @@ static int zmr_filedata(FAR struct zm_state_s *pzm)
 
       if (pzm->nerrors > CONFIG_SYSTEM_ZMODEM_MAXERRORS)
         {
-          zmdbg("PSTATE %d:%d->%d.%d\n",
+          zmdbg("PSTATE %d:%d->%d:%d\n",
                 pzm->pstate, pzm->psubstate, PSTATE_DATA, PDATA_READ);
 
           /* Revert to the IDLE state and send the cancel string */
@@ -746,7 +748,7 @@ static int zmr_filedata(FAR struct zm_state_s *pzm)
         }
       else
         {
-          zmdbg("PSTATE %d:%d->%d.%d\n",
+          zmdbg("PSTATE %d:%d->%d:%d\n",
                 pzm->pstate, pzm->psubstate, PSTATE_IDLE, PIDLE_ZPAD);
           zmdbg("ZMR_STATE %d->%d\n",  pzm->state, ZMR_READREADY);
 
@@ -769,7 +771,7 @@ static int zmr_filedata(FAR struct zm_state_s *pzm)
       /* Could not write to the the file. */
 
       zmdbg("ERROR: Write to file failed: %d\n", errorcode);
-      zmdbg("PSTATE %d:%d->%d.%d\n",
+      zmdbg("PSTATE %d:%d->%d:%d\n",
              pzm->pstate, pzm->psubstate, PSTATE_IDLE, PIDLE_ZPAD);
       zmdbg("ZMR_STATE %d->%d\n",  pzm->state, ZMR_FINISH);
 
@@ -781,16 +783,17 @@ static int zmr_filedata(FAR struct zm_state_s *pzm)
       return zmr_fileerror(pzmr, ZFERR, (uint32_t)errorcode);
     }
 
-  zmdbg("offset: %ld nchars: %d\n", (unsigned long)pzmr->offset, pzm->pktlen);
+  zmdbg("offset: %ld nchars: %d pkttype: %02x\n",
+        (unsigned long)pzmr->offset, pzm->pktlen, pzm->pkttype);
 
   pzmr->offset += pzm->pktlen;
-  zmdbg("%ld bytes received\n", (unsigned long)pzmr->offset);
+  zmdbg("Bytes received: %ld\n", (unsigned long)pzmr->offset);
 
   /* If this was the last data subpacket, leave data mode */
 
-  if (pzmr->pkttype == ZCRCE || pzmr->pkttype == ZCRCW)
+  if (pzm->pkttype == ZCRCE || pzm->pkttype == ZCRCW)
     {
-      zmdbg("PSTATE %d:%d->%d.%d: ZCRCE|ZCRCW\n",
+      zmdbg("PSTATE %d:%d->%d:%d: ZCRCE|ZCRCW\n",
             pzm->pstate, pzm->psubstate, PSTATE_IDLE, PIDLE_ZPAD);
       zmdbg("ZMR_STATE %d->%d\n",  pzm->state, ZMR_READREADY);
 
@@ -815,7 +818,7 @@ static int zmr_filedata(FAR struct zm_state_s *pzm)
    *   ZCRCE:  End of file, no response
    */
 
-  if (pzmr->pkttype == ZCRCQ || pzmr->pkttype == ZCRCW)
+  if (pzm->pkttype == ZCRCQ || pzm->pkttype == ZCRCW)
     {
       zmdbg("Send ZACK\n");
 
@@ -961,7 +964,7 @@ static int zmr_zfin(FAR struct zm_state_s *pzm)
    * meaning that we are all done.
    */
 
-  zmdbg("PSTATE %d:%d->%d.%d:  Send ZFIN\n",
+  zmdbg("PSTATE %d:%d->%d:%d:  Send ZFIN\n",
          pzm->pstate, pzm->psubstate, PSTATE_IDLE, PIDLE_ZPAD);
   zmdbg("ZMR_STATE %d\n", pzm->state);
 
@@ -999,6 +1002,8 @@ static int zmr_finto(FAR struct zm_state_s *pzm)
    */
 
   pzmr->ntimeouts++;
+  pzm->flags  &= ~ZM_FLAG_OO; /* No longer expect "OO" */
+
   zmdbg("ZMR_STATE %d: %d send timeouts\n", pzm->state, pzmr->ntimeouts);
 
   /* And terminate the reception with a timeout error */
@@ -1007,7 +1012,7 @@ static int zmr_finto(FAR struct zm_state_s *pzm)
 }
 
 /****************************************************************************
- * Name: zmr_finto
+ * Name: zmr_oo
  *
  * Description:
  *   Received "OO" in the ZMR_FINISH state.  We are finished!
@@ -1103,6 +1108,7 @@ static int zmr_error(FAR struct zm_state_s *pzm)
         pzm->state, pzm->hdrdata[0]);
 
   pzm->flags |= ZM_FLAG_WAIT;
+  pzm->flags &= ~ZM_FLAG_OO;   /* In case we get here from ZMR_FINISH */
   return OK;
 }
 
@@ -1473,7 +1479,7 @@ static int zmr_fileerror(FAR struct zmr_state_s *pzmr, uint8_t type,
 
   /* Set the state back to IDLE to abort the transfer */
 
-  zmdbg("PSTATE %d:%d->%d.%d\n",
+  zmdbg("PSTATE %d:%d->%d:%d\n",
         pzmr->cmn.pstate, pzmr->cmn.psubstate, PSTATE_IDLE, PIDLE_ZPAD);
 
   pzmr->cmn.pstate    = PSTATE_IDLE;
diff --git a/apps/system/zmodem/zm_send.c b/apps/system/zmodem/zm_send.c
index edb34204d..9bcc23581 100644
--- a/apps/system/zmodem/zm_send.c
+++ b/apps/system/zmodem/zm_send.c
@@ -614,6 +614,7 @@ static int zms_command(FAR struct zm_state_s *pzm)
 static int zms_message(FAR struct zm_state_s *pzm)
 {
   zmdbg("ZMS_STATE %d\n", pzm->state);
+
   zm_readstate(pzm);
   return OK;
 }
@@ -629,6 +630,7 @@ static int zms_message(FAR struct zm_state_s *pzm)
 static int zms_stderrdata(FAR struct zm_state_s *pzm)
 {
   zmdbg("ZMS_STATE %d\n", pzm->state);
+
   pzm->pktbuf[pzm->pktlen] = '\0';
   fprintf(stderr, "Message: %s", (char*)pzm->pktbuf);
   return OK;
@@ -647,6 +649,7 @@ static int zms_stderrdata(FAR struct zm_state_s *pzm)
 static int zms_initdone(FAR struct zm_state_s *pzm)
 {
   zmdbg("ZMS_STATE %d->%d\n", pzm->state, ZMS_DONE);
+
   pzm->state = ZMS_DONE;
   return ZM_XFRDONE;
 }
diff --git a/apps/system/zmodem/zm_state.c b/apps/system/zmodem/zm_state.c
index 007685981..261dba388 100644
--- a/apps/system/zmodem/zm_state.c
+++ b/apps/system/zmodem/zm_state.c
@@ -154,7 +154,7 @@ static int zm_event(FAR struct zm_state_s *pzm, int event)
 
 static int zm_nakhdr(FAR struct zm_state_s *pzm)
 {
-  zmdbg("PSTATE %d:%d->%d.%d: NAKing\n",
+  zmdbg("PSTATE %d:%d->%d:%d: NAKing\n",
         pzm->pstate, pzm->psubstate, PSTATE_IDLE, PIDLE_ZPAD);
 
   /* Revert to the IDLE state */
@@ -180,7 +180,7 @@ static int zm_hdrevent(FAR struct zm_state_s *pzm)
   zmdbg("Received type: %d data: %02x %02x %02x %02x\n",
         pzm->hdrdata[0],
         pzm->hdrdata[1], pzm->hdrdata[2], pzm->hdrdata[3], pzm->hdrdata[4]);
-  zmdbg("PSTATE %d:%d->%d.%d\n",
+  zmdbg("PSTATE %d:%d->%d:%d\n",
         pzm->pstate, pzm->psubstate, PSTATE_IDLE, PIDLE_ZPAD);
 
   /* Revert to the IDLE state */
@@ -231,7 +231,7 @@ static int zm_hdrevent(FAR struct zm_state_s *pzm)
 static int zm_dataevent(FAR struct zm_state_s *pzm)
 {
   zmdbg("Received type: %d length: %d\n", pzm->pkttype, pzm->pktlen);
-  zmdbg("PSTATE %d:%d->%d.%d\n",
+  zmdbg("PSTATE %d:%d->%d:%d\n",
         pzm->pstate, pzm->psubstate, PSTATE_IDLE, PIDLE_ZPAD);
 
   /* Revert to the IDLE state */
@@ -255,6 +255,12 @@ static int zm_dataevent(FAR struct zm_state_s *pzm)
         {
           pzm->flags |= ZM_FLAG_CRKOK;
         }
+
+      /* Adjust the back length to exclude the packet type length of the 4-
+       * byte checksum.
+       */
+
+      pzm->pktlen -= 5;
     }
   else
     {
@@ -270,8 +276,16 @@ static int zm_dataevent(FAR struct zm_state_s *pzm)
         {
           pzm->flags |= ZM_FLAG_CRKOK;
         }
+
+     /* Adjust the back length to exclude the packet type length of the 2-
+      * byte checksum.
+      */
+
+      pzm->pktlen -= 3;
     }
 
+  /* Then handle the data received event */
+
   return zm_event(pzm, ZME_DATARCVD);
 }
 
@@ -296,7 +310,7 @@ static int zm_idle(FAR struct zm_state_s *pzm, uint8_t ch)
       {
         /* The ZDLE character is expected next */
 
-        zmdbg("PSTATE %d:%d->%d.%d\n",
+        zmdbg("PSTATE %d:%d->%d:%d\n",
               pzm->pstate, pzm->psubstate, pzm->pstate, PIDLE_ZDLE);
 
         pzm->psubstate = PIDLE_ZDLE;
@@ -313,7 +327,7 @@ static int zm_idle(FAR struct zm_state_s *pzm, uint8_t ch)
 
       if (pzm->psubstate == PIDLE_ZDLE)
         {
-          zmdbg("PSTATE %d:%d->%d.%d\n",
+          zmdbg("PSTATE %d:%d->%d:%d\n",
                 pzm->pstate, pzm->psubstate, PSTATE_HEADER, PHEADER_FORMAT);
 
           pzm->flags    &= ~ZM_FLAG_OO;
@@ -323,7 +337,7 @@ static int zm_idle(FAR struct zm_state_s *pzm, uint8_t ch)
         }
       else
         {
-          zmdbg("PSTATE %d:%d->%d.%d\n",
+          zmdbg("PSTATE %d:%d->%d:%d\n",
                 pzm->pstate, pzm->psubstate, pzm->pstate, PIDLE_ZPAD);
 
           pzm->psubstate = PIDLE_ZPAD;
@@ -340,27 +354,28 @@ static int zm_idle(FAR struct zm_state_s *pzm, uint8_t ch)
        * case if not.
        */
 
-      if ((pzm->state & ZM_FLAG_OO) != 0)
+      if ((pzm->flags & ZM_FLAG_OO) != 0)
         {
           /* Yes... did we receive an 'O' before this one? */
 
-          if (pzm->psubstate != PIDLE_OO)
+          if (pzm->psubstate == PIDLE_OO)
             {
               /* This is the second 'O' of "OO". the receiver operation is
                * finished.
                */
 
-              zmdbg("PSTATE %d:%d->%d.%d\n",
+              zmdbg("PSTATE %d:%d->%d:%d\n",
                     pzm->pstate, pzm->psubstate, pzm->pstate, PIDLE_ZPAD);
 
+              pzm->flags    &= ~ZM_FLAG_OO;
               pzm->psubstate = PIDLE_ZPAD;
               return zm_event(pzm, ZME_OO);
             }
           else
             {
-              /* No... this is the first 'O' that we have seen */
+              /* No... then this is the first 'O' that we have seen */
 
-              zmdbg("PSTATE %d:%d->%d.%d\n",
+              zmdbg("PSTATE %d:%d->%d:%d\n",
                     pzm->pstate, pzm->psubstate, pzm->pstate, PIDLE_OO);
 
               pzm->psubstate = PIDLE_OO;
@@ -373,7 +388,7 @@ static int zm_idle(FAR struct zm_state_s *pzm, uint8_t ch)
     default:
       if (pzm->psubstate != PIDLE_ZPAD)
         {
-          zmdbg("PSTATE %d:%d->%d.%d\n",
+          zmdbg("PSTATE %d:%d->%d:%d\n",
                 pzm->pstate, pzm->psubstate, pzm->pstate, PIDLE_ZPAD);
 
           pzm->psubstate = PIDLE_ZPAD;
@@ -894,7 +909,7 @@ int zm_datapump(FAR struct zm_state_s *pzm)
                 {
                   /* Yes... a timeout occurred */
 
-                  zm_timeout(pzm);
+                  ret = zm_timeout(pzm);
                 }
 
               /* No.. then just ignore the EINTR. */
@@ -940,7 +955,7 @@ int zm_datapump(FAR struct zm_state_s *pzm)
 
 void zm_readstate(FAR struct zm_state_s *pzm)
 {
-  zmdbg("PSTATE %d:%d->%d.%d\n",
+  zmdbg("PSTATE %d:%d->%d:%d\n",
         pzm->pstate, pzm->psubstate, PSTATE_DATA, PDATA_READ);
 
   pzm->pstate    = PSTATE_DATA;