Misc NXFFS bugfixes

git-svn-id: svn://svn.code.sf.net/p/nuttx/code/trunk@3559 42af7a65-404d-4744-a932-0658087f49c3

5 files changed, 149 insertions, 121 deletions

diff --git a/nuttx/fs/nxffs/nxffs.h b/nuttx/fs/nxffs/nxffs.h
index 6d56959c4..f2e9572e9 100644
--- a/nuttx/fs/nxffs/nxffs.h
+++ b/nuttx/fs/nxffs/nxffs.h
@@ -754,12 +754,14 @@ extern FAR struct nxffs_ofile_s *nxffs_findofile(FAR struct nxffs_volume_s *volu
  * Name: nxffs_wrinode
  *
  * Description:
- *   Write the inode header and inode file name to FLASH.  This is done in
- *   two contexts:
+ *   Write the inode header (only to FLASH.  This is done in two contexts:
  *
  *   1. When an inode is closed, or
  *   2. As part of the file system packing logic when an inode is moved.
  *
+ * Note that in either case, the inode name has already been written to
+ * FLASH.
+ *
  * Input parameters
  *   volume - Describes the NXFFS volume
  *   entry  - Describes the indoe header to write
diff --git a/nuttx/fs/nxffs/nxffs_dump.c b/nuttx/fs/nxffs/nxffs_dump.c
index fc60e8a5d..fbd3807d3 100644
--- a/nuttx/fs/nxffs/nxffs_dump.c
+++ b/nuttx/fs/nxffs/nxffs_dump.c
@@ -81,7 +81,6 @@ struct nxffs_blkinfo_s
 
 static const char g_hdrformat[] = "  BLOCK:OFFS  TYPE  STATE   LENGTH\n";
 static const char g_format[]    = "  %5d:%-5d %s %s %5d\n";
-static const char g_blkformat[] = "--%5d:%-5d %s %s %5d\n";
 
 /****************************************************************************
  * Private Functions
@@ -292,7 +291,7 @@ static inline void nxffs_analyze(FAR struct nxffs_blkinfo_s *blkinfo)
   blkhdr = (FAR struct nxffs_block_s *)blkinfo->buffer;
   if (memcmp(blkhdr->magic, g_blockmagic, NXFFS_MAGICSIZE) != 0)
     {
-      fdbg(g_blkformat, blkinfo->block, 0, "BLOCK", "NO FRMT",
+      fdbg(g_format, blkinfo->block, 0, "BLOCK", "NO FRMT",
            blkinfo->geo.blocksize);
     }
   else if (blkhdr->state == BLOCK_STATE_GOOD)
@@ -301,24 +300,26 @@ static inline void nxffs_analyze(FAR struct nxffs_blkinfo_s *blkinfo)
       size_t nerased = nxffs_erased(blkinfo->buffer + SIZEOF_NXFFS_BLOCK_HDR, datsize);
       if (nerased == datsize)
         {
-          fdbg(g_blkformat, blkinfo->block, 0, "BLOCK", "ERASED ",
+          fdbg(g_format, blkinfo->block, 0, "BLOCK", "ERASED ",
                blkinfo->geo.blocksize);
           return;
         }
+#if 0 /* Too much output, to little information */
       else
         {
-          fdbg(g_blkformat, blkinfo->block, 0, "BLOCK", "IN USE ",
+          fdbg(g_format, blkinfo->block, 0, "BLOCK", "IN USE ",
                blkinfo->geo.blocksize);
         }
+#endif
     }
   else if (blkhdr->state == BLOCK_STATE_BAD)
     {
-      fdbg(g_blkformat, blkinfo->block, 0, "BLOCK", "BAD    ",
+      fdbg(g_format, blkinfo->block, 0, "BLOCK", "BAD    ",
            blkinfo->geo.blocksize);
     }
   else
     {
-      fdbg(g_blkformat, blkinfo->block, 0, "BLOCK", "CORRUPT",
+      fdbg(g_format, blkinfo->block, 0, "BLOCK", "CORRUPT",
            blkinfo->geo.blocksize);
     }
 
@@ -342,8 +343,9 @@ static inline void nxffs_analyze(FAR struct nxffs_blkinfo_s *blkinfo)
               nbytes = nxffs_analyzeinode(blkinfo, hdrndx);
               if (nbytes > 0)
                 {
-                  i = hdrndx + nbytes;
+                  i = hdrndx + nbytes - 1;
                 }
+              inndx = 0;
             }
         }
       else if (ch == g_datamagic[datndx])
@@ -355,11 +357,11 @@ static inline void nxffs_analyze(FAR struct nxffs_blkinfo_s *blkinfo)
             {
               hdrndx = i - NXFFS_MAGICSIZE + 1;
               nbytes = nxffs_analyzedata(blkinfo, hdrndx);
-              nbytes = nxffs_analyzeinode(blkinfo, hdrndx);
               if (nbytes > 0)
                 {
-                  i = hdrndx + nbytes;
+                  i = hdrndx + nbytes - 1;
                 }
+              datndx = 0;
             }
         }
     }
diff --git a/nuttx/fs/nxffs/nxffs_open.c b/nuttx/fs/nxffs/nxffs_open.c
index 02c15fa43..6d8e30f1d 100644
--- a/nuttx/fs/nxffs/nxffs_open.c
+++ b/nuttx/fs/nxffs/nxffs_open.c
@@ -111,7 +111,7 @@ static struct nxffs_wrfile_s g_wrfile;
  *
  *   On successful return the following are also valid:
  *
- *     wrfile->ofile.entry.hoffset - Flash offset to candidate header position
+ *     wrfile->ofile.entry.hoffset - FLASH offset to candidate header position
  *     volume->ioblock - Read/write block number of the block containing the
  *       header position
  *     volume->iooffset - The offset in the block to the candidate header
@@ -165,7 +165,7 @@ static inline int nxffs_hdrpos(FAR struct nxffs_volume_s *volume,
  *
  *   On successful return the following are also valid:
  *
- *     wrfile->ofile.entry.noffset - Flash offset to candidate name position
+ *     wrfile->ofile.entry.noffset - FLASH offset to candidate name position
  *     volume->ioblock - Read/write block number of the block containing the
  *       name position
  *     volume->iooffset - The offset in the block to the candidate name
@@ -227,7 +227,7 @@ static inline int nxffs_nampos(FAR struct nxffs_volume_s *volume,
  *
  *   On successful return the following are also valid:
  *
- *     wrfile->ofile.entry.hoffset - Flash offset to candidate header position
+ *     wrfile->ofile.entry.hoffset - FLASH offset to candidate header position
  *     volume->ioblock - Read/write block number of the block containing the
  *       header position
  *     volume->iooffset - The offset in the block to the candidate header
@@ -288,7 +288,7 @@ static inline int nxffs_hdrerased(FAR struct nxffs_volume_s *volume,
  *
  *   On successful return the following are also valid:
  *
- *     wrfile->ofile.entry.noffset - Flash offset to candidate name position
+ *     wrfile->ofile.entry.noffset - FLASH offset to candidate name position
  *     volume->ioblock - Read/write block number of the block containing the
  *       name position
  *     volume->iooffset - The offset in the block to the candidate name
@@ -316,6 +316,63 @@ static inline int nxffs_namerased(FAR struct nxffs_volume_s *volume,
 }
 
 /****************************************************************************
+ * Name: nxffs_wrname
+ *
+ * Description:
+ *   Write the inode name to cache at the position verified by
+ *   nxffs_namerased().
+ *
+ *   On entry it assumes:
+ *
+ *     entry->noffset - FLASH offset to final name position
+ *     volume->ioblock  - Read/write block number of the block containing the
+ *       name position
+ *     volume->iooffset - The offset in the block to the candidate name
+ *       position.
+ *
+ * Input Parameters:
+ *   volume - Describes the NXFFS volume
+ *   entry - Describes the entry to be written. 
+ *
+ * Returned Value:
+ *   Zero is returned on success.  Otherwise, a negated errno value is
+ *   returned indicating the nature of the failure.
+ *
+ ****************************************************************************/
+
+static inline int nxffs_wrname(FAR struct nxffs_volume_s *volume,
+                               FAR struct nxffs_entry_s *entry,
+                               int namlen)
+{
+  int ret;
+
+  /* Seek to the inode name position and assure that it is in the volume
+   * cache.
+   */
+
+  nxffs_ioseek(volume, entry->noffset);
+  ret = nxffs_rdcache(volume, volume->ioblock);
+  if (ret < 0)
+    {
+      fdbg("Failed to read inode name block %d: %d\n",
+           volume->ioblock, -ret);
+      return ret;
+    }
+
+  /* Copy the inode name to the volume cache and write the inode name block */
+
+  memcpy(&volume->cache[volume->iooffset], entry->name, namlen);
+  ret = nxffs_wrcache(volume);
+  if (ret < 0)
+    {
+      fdbg("Failed to write inode header block %d: %d\n",
+           volume->ioblock, -ret);
+    }
+
+  return ret;
+}
+
+/****************************************************************************
  * Name: nxffs_wropen
  *
  * Description:
@@ -517,8 +574,7 @@ static inline int nxffs_wropen(FAR struct nxffs_volume_s *volume,
     }
 
   /* Loop until the inode name is configured or until a failure occurs.
-   * Note that nothing is written to FLASH.  The inode name is not
-   * written until the file is closed.
+   * Note that nothing is written to FLASH. 
    */
 
   for (;;)
@@ -535,8 +591,20 @@ static inline int nxffs_wropen(FAR struct nxffs_volume_s *volume,
           ret = nxffs_namerased(volume, wrfile, namlen);
           if (ret == OK)
             {
-              /* Valid memory for the inode header was found.  Break out of
-               * the loop.
+              /* Valid memory for the inode header was found.  Write the
+               * inode name to this location.
+               */
+
+              ret = nxffs_wrname(volume, &wrfile->ofile.entry, namlen);
+              if (ret < 0)
+                {
+                  fdbg("Failed to write the inode name: %d\n", -ret);
+                  goto errout_with_name;
+                }
+
+              /* Then just break out of the loop reporting success.  Note
+               * that the alllocated inode name string is retained; it
+               * will be needed later to calculate the inode CRC.
                */
 
               break;
@@ -1028,12 +1096,14 @@ errout:
  * Name: nxffs_wrinode
  *
  * Description:
- *   Write the inode header and inode file name to FLASH.  This is done in
- *   two contexts:
+ *   Write the inode header (only to FLASH.  This is done in two contexts:
  *
  *   1. When an inode is closed, or
  *   2. As part of the file system packing logic when an inode is moved.
  *
+ * Note that in either case, the inode name has already been written to
+ * FLASH.
+ *
  * Input parameters
  *   volume - Describes the NXFFS volume
  *   entry  - Describes the indoe header to write
@@ -1044,25 +1114,16 @@ errout:
  *
  ****************************************************************************/
 
-int nxffs_wrinode(FAR struct nxffs_volume_s *volume, FAR struct nxffs_entry_s *entry)
+int nxffs_wrinode(FAR struct nxffs_volume_s *volume,
+                  FAR struct nxffs_entry_s *entry)
 {
   FAR struct nxffs_inode_s *inode;
-  off_t namblock;
-  uint16_t namoffset;
   uint32_t crc;
   int namlen;
   int ret;
 
-  /* Write the inode header to FLASH.  First get the block where we will
-   * write the file name.
-   */
-
-  nxffs_ioseek(volume, entry->noffset);
-  namblock  = volume->ioblock;
-  namoffset = volume->iooffset;
-
-  /* Now seek to the inode header position and assure that it is in the
-   * volume cache.
+  /* Seek to the inode header position and assure that it is in the volume
+   * cache.
    */
 
   nxffs_ioseek(volume, entry->hoffset);
@@ -1103,46 +1164,15 @@ int nxffs_wrinode(FAR struct nxffs_volume_s *volume, FAR struct nxffs_entry_s *e
   inode->state = INODE_STATE_FILE;
   nxffs_wrle32(inode->crc, crc);
 
-  /* Are the inode header and the inode name in the same block?  Normally,
-   * they will be in the same block.  However, they could potentially be
-   * far apart due to intervening bad blocks.
-   */
-
-  if (volume->ioblock != namblock)
-    {
-      /* Write the block with the inode header */
-
-      ret = nxffs_wrcache(volume);
-      if (ret < 0)
-        {
-          fdbg("Failed to write inode header block %d: %d\n",
-               volume->ioblock, -ret);
-          goto errout;
-        }
-
-      /* Make sure the that block containing the inode name is in the cache */
+  /* Write the block with the inode header */
 
-      volume->ioblock  = namblock;
-      volume->iooffset = namoffset;
-      ret = nxffs_rdcache(volume, volume->ioblock);
-      if (ret < 0)
-        {
-          fdbg("Failed to read inode name block %d: %d\n",
-               volume->ioblock, -ret);
-          goto errout;
-        }
-    }
-
-  /* Finally, copy the inode name to the cache and write the inode name block */
-
-  memcpy(&volume->cache[namoffset], entry->name, namlen);
   ret = nxffs_wrcache(volume);
   if (ret < 0)
     {
       fdbg("Failed to write inode header block %d: %d\n",
            volume->ioblock, -ret);
     }
-  
+
   /* The volume is now available for other writers */
 
 errout:
diff --git a/nuttx/fs/nxffs/nxffs_pack.c b/nuttx/fs/nxffs/nxffs_pack.c
index 5815d0a1a..9a80b41a9 100644
--- a/nuttx/fs/nxffs/nxffs_pack.c
+++ b/nuttx/fs/nxffs/nxffs_pack.c
@@ -475,7 +475,14 @@ static int nxffs_destsetup(FAR struct nxffs_volume_s *volume,
           return -ENOSPC;
         }
 
-      /* Yes.. reserve space for the inode name (but don't write it yet) */
+      /* Yes.. Write the inode name to the volume packing buffer now, but do
+       * not free the name string memory yet; it will be needed later to\
+       * calculate the header CRC.
+       */
+
+      memcpy(&volume->pack[pack->iooffset], pack->dest.entry.name, namlen);
+
+      /* Reserve space for the inode name  */
 
       pack->dest.entry.noffset = nxffs_packtell(volume, pack);
       pack->iooffset += namlen;
@@ -548,7 +555,9 @@ static int nxffs_destsetup(FAR struct nxffs_volume_s *volume,
  * Name: nxffs_wrinodehdr
  *
  * Description:
- *   Write the destination inode header to FLASH.
+ *   Write the destination inode header (only) to FLASH.  Note that the inode
+ *   name has already been written to FLASH (thus greatly simplifying the
+ *   the complexity of this operation).
  *
  * Input Parameters:
  *   volume - The volume to be packed
@@ -565,64 +574,38 @@ static int nxffs_wrinodehdr(FAR struct nxffs_volume_s *volume,
 {
   FAR struct nxffs_inode_s *inode;
   off_t ioblock;
-  off_t namblock;
   uint16_t iooffset;
-  uint16_t namoffset;
   uint32_t crc;
   int namlen;
-  int ret;
 
-  /* Get positions corresponding to the inode header and inode name positions */
+  /* Get seek positions corresponding to the inode header location */
 
   ioblock   = nxffs_getblock(volume, pack->dest.entry.hoffset);
   iooffset  = nxffs_getoffset(volume, pack->dest.entry.hoffset, ioblock);
 
-  namblock  = nxffs_getblock(volume, pack->dest.entry.noffset);
-  namoffset = nxffs_getoffset(volume, pack->dest.entry.noffset, namblock);
-
   /* The inode header is not written until all of the inode data has been
-   * packed into its new location.  As a result, there are three possibilities:
+   * packed into its new location.  As a result, there are two possibilities:
    *
    * 1. The inode header lies in the current, unwritten erase block,
    * 2. The inode header resides in an earlier erase block and has already
-   *    been written to FLASH, but the inode name resides within the erase
-   *    block and has not been written to FLASH, or
-   * 3. The inode header resides in an earlier erase block and has already
-   *    been written to FLASH (most likely case for files larger than an
-   *    erase block).
+   *    been written to FLASH.
+   *
+   * Recall that the inode name has already been written to FLASH.  If that
+   * were not the case, then there would be other complex possibilities.
    *
-   * Case 2 & 3: Does the inode header reside in a block before the beginning
+   * Case 2: Does the inode header reside in a block before the beginning
    * of the current erase block?
    */
 
   if (ioblock < pack->block0)
     {
-      /* Does the inode name also reside in a block before the beginning of
-       * the current erase block?
+      /* Case 2:  The inode header lies in an earlier erase block that has
+       * already been written to FLASH.  In this case, if we are very
+       * careful, we can just use the standard routine to write the inode
+       * header that is called during the normal file close operation:
        */
 
-      if (namblock < pack->block0)
-        {
-          /* Yes.. this is case 3:  Both the inode block header and the inode
-           * name lie in an earlier erase block that has already been written
-           * to FLASH.  In this case, if we are very careful, we can just use
-           * the standard routine to write the inode header that is called
-           * during the normal file close operation:
-           */
-
-          ret = nxffs_wrinode(volume, &pack->dest.entry);
-          return ret;
-        }
-      else
-        {
-          /* Case 2:  The inode header lies in an earlier erase block that
-           * has been written to FLASH but the inode name is in the cache and
-           * still unwritten.
-           */
-
-#warning "Missing logic"
-return -ENOSYS;
-        }
+      return nxffs_wrinode(volume, &pack->dest.entry);
     }
 
   /* Cases 1:  Both the inode header and name are in the unwritten cache memory. */
@@ -656,11 +639,6 @@ return -ENOSYS;
   inode->state = INODE_STATE_FILE;
   nxffs_wrle32(inode->crc, crc);
 
-  /* Write the inode name */
-
-  namoffset += (namblock - pack->block0) * volume->geo.blocksize;
-  memcpy(&volume->pack[namoffset], pack->dest.entry.name, namlen);
-
   /* Reset the dest inode information */
 
   nxffs_freeentry(&pack->dest.entry);
@@ -692,14 +670,14 @@ static void nxffs_wrdathdr(FAR struct nxffs_volume_s *volume,
   uint16_t iooffset;
   uint32_t crc;
 
-  /* Get the offset in the block corresponding to the location of the inode
-   * header.  NOTE:  This must lie in the same block as we currently have
+  /* Get the offset in the block corresponding to the location of the data
+   * block header.  NOTE:  This must lie in the same block as we currently have
    * buffered.
    */
 
-  ioblock  = nxffs_getblock(volume, pack->dest.entry.hoffset);
-  iooffset = nxffs_getoffset(volume, pack->dest.entry.hoffset, ioblock);
-  DEBUGASSERT(ioblock == pack->ioblock);
+  ioblock  = nxffs_getblock(volume, pack->dest.blkoffset);
+  iooffset = nxffs_getoffset(volume, pack->dest.blkoffset, ioblock);
+  DEBUGASSERT(pack->dest.blkoffset && ioblock == pack->ioblock);
 
   /* Write the data block header to memory */
 
@@ -915,7 +893,7 @@ static inline int nxffs_packblock(FAR struct nxffs_volume_s *volume,
 
           /* Yes.. find the next data block in the source input stream. */
 
-          offset = pack->src.blkoffset + pack->src.blklen;
+          offset = pack->src.blkoffset + SIZEOF_NXFFS_BLOCK_HDR + pack->src.blklen;
           ret    = nxffs_nextblock(volume, offset, &blkentry);
           if (ret < 0)
             {
@@ -1082,12 +1060,25 @@ int nxffs_pack(FAR struct nxffs_volume_s *volume)
               }
          }
 
+      /* We now have an in-memory image of how we want this erase block to
+       * appear. Now it is safe to erase the block.
+       */
+
+      ret = MTD_ERASE(volume->mtd, eblock, 1);
+      if (ret < 0)
+        {
+          fdbg("Failed to erase block %d [%d]: %d\n",
+               eblock, pack.block0, -ret);
+          goto errout_with_pack;
+        }
+
       /* Write the packed I/O block to FLASH */
 
       ret = MTD_BWRITE(volume->mtd, pack.block0, volume->blkper, volume->pack);
       if (ret < 0)
         {
-          fdbg("Failed to write erase block %d: %d\n", eblock, -ret);
+          fdbg("Failed to write erase block %d [%]: %d\n",
+               eblock, pack.block0, -ret);
           goto errout_with_pack;
         }
     }
diff --git a/nuttx/fs/nxffs/nxffs_read.c b/nuttx/fs/nxffs/nxffs_read.c
index 0c2768c6a..36f2166bb 100644
--- a/nuttx/fs/nxffs/nxffs_read.c
+++ b/nuttx/fs/nxffs/nxffs_read.c
@@ -344,7 +344,7 @@ int nxffs_nextblock(FAR struct nxffs_volume_s *volume, off_t offset,
                * FLASH seek location.
                */
 
-              blkentry->hoffset = nxffs_iotell(volume) - 4;
+              blkentry->hoffset = nxffs_iotell(volume) - NXFFS_MAGICSIZE;
 
               /* Read the block header and verify the block at that address */
 
@@ -356,8 +356,11 @@ int nxffs_nextblock(FAR struct nxffs_volume_s *volume, off_t offset,
                   return OK;
                 }
 
-              /* False alarm.. keep looking */
+              /* False alarm.. Restore the volume cache position (that was
+               * destroyed by nxfs_rdblkhdr()) and keep looking.
+               */
 
+              nxffs_ioseek(volume, blkentry->hoffset + NXFFS_MAGICSIZE);
               nmagic = 0;
             }
         }