fs/spiffs: Need to flush cache to FLASH when closing file. Also updates TODO list.

TODO

@@ -1,4 +1,4 @@
-NuttX TODO List (Last updated September 26, 2018)
+NuttX TODO List (Last updated September 29, 2018)
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 This file summarizes known NuttX bugs, limitations, inconsistencies with
@@ -23,7 +23,7 @@ nuttx/:
   (4)  USB (drivers/usbdev, drivers/usbhost)
   (2)  Other drivers (drivers/)
  (12)  Libraries (libc/, libm/)
- (10)  File system/Generic drivers (fs/, drivers/)
+ (11)  File system/Generic drivers (fs/, drivers/)
  (10)  Graphics Subsystem (graphics/)
   (1)  Build system / Toolchains
   (3)  Linux/Cywgin simulation (arch/sim)
@@ -2058,6 +2058,40 @@ o File system / Generic drivers (fs/, drivers/)
                   ignored by readdir() logic.  This is why the file does
                   not appear in the 'ls'.
 
+  Title:       SILENT SPIFFS FILE TRUNCATION
+  Description: Under certain corner case conditions, SPIFFS will truncate
+               files.  All of the writes to the file will claim that the
+               data has been written but after the file is closed, it may
+               be a little shorter than expected.
+
+               This is due to how the caching is implemented in SPIFFS:
+
+               1. On each write, the data is not written to the FLASH but
+                  rather to an internal cache in memory.
+               2. When the a write causes the cache to become full, the
+                  content of cache is flushed to memory.  If that flush
+                  fails because the FLASH has become full, write will
+                  return the file system full error (ENOSPC).
+               3. The cache is also flushed when the file is closed (or
+                  when fsync() is called).  These will also fail if the
+                  file system becomes full.
+
+               The problem is when the file is closed, the final file
+               size could be smaller than the number of successful writes
+               to the file.
+
+               This error is probably not so significant in a real world
+               file system usage, but does cause the test at
+               apps/examples/fstest to fail.  That test fails with an error
+               of a "Partial Read" because the file being read is smaller
+               than number bytes written to the file.  This happens very
+               rarely on only may 1 out of 100 files and only if you write
+               constantly until the file system is full.  The
+               configs/sim/spiffs test can used to demonstrate this error.
+  Status:      Open
+  Priority:    Medium.  It is certain a file system failure, but I think that
+               the exposure in real world uses cases is very small.
+
 o Graphics Subsystem (graphics/)
   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 

fs/spiffs/src/spiffs.h

@@ -246,7 +246,7 @@ int spiffs_find_fobj_byobjid(FAR struct spiffs_s *fs, int16_t objid,
                              FAR struct spiffs_file_s **ppfobj);
 
 /****************************************************************************
- * Name: spiffs_fflush_cache
+ * Name: spiffs_fobj_flush
  *
  * Description:
  *   Checks if there are any cached writes for the object ID associated with
@@ -262,8 +262,8 @@ int spiffs_find_fobj_byobjid(FAR struct spiffs_s *fs, int16_t objid,
  *
  ****************************************************************************/
 
-ssize_t spiffs_fflush_cache(FAR struct spiffs_s *fs,
+ssize_t spiffs_fobj_flush(FAR struct spiffs_s *fs,
-                            FAR struct spiffs_file_s *fobj);
+                          FAR struct spiffs_file_s *fobj);
 
 /****************************************************************************
  * Name: spiffs_fobj_write

fs/spiffs/src/spiffs_vfs.c

@@ -511,6 +511,7 @@ static int spiffs_close(FAR struct file *filep)
   FAR struct inode *inode;
   FAR struct spiffs_s *fs;
   FAR struct spiffs_file_s *fobj;
+  int ret;
 
   finfo("filep=%p\n", filep);
   DEBUGASSERT(filep->f_priv != NULL && filep->f_inode != NULL);
@@ -547,9 +548,24 @@ static int spiffs_close(FAR struct file *filep)
 
   if (fobj->crefs == 0)
     {
+      ssize_t nflushed;
+
+      /* Flush any cached writes for the file object being closed.
+       * This could result in an ENOSPC error being reported if the
+       * cache could not flushed to FALSH (and the file will appear to
+       * to have been truncated).
+       */
+
+      nflushed = spiffs_fobj_flush(fs, fobj);
+      if (nflushed < 0)
+        {
+          ferr("ERROR: spiffs_fobj_flush() failed: %d\n", ret);
+          ret = (int)nflushed;
+        }
+
       /* Free the file object while we hold the lock?  Weird but this
-       * should be safe because the object is unlinked and could not
+       * should be safe because the object is does not have any other
-       * have any other references.
+       * references.
        *
        * If the file was unlinked while it was opened, then now would be
        * the time to perform the unlink operation.
@@ -561,7 +577,7 @@ static int spiffs_close(FAR struct file *filep)
   /* Release the lock on the file system */
 
   spiffs_unlock_volume(fs);
-  return OK;
+  return ret;
 }
 
 /****************************************************************************
@@ -820,6 +836,7 @@ static off_t spiffs_seek(FAR struct file *filep, off_t offset, int whence)
   FAR struct inode *inode;
   FAR struct spiffs_s *fs;
   FAR struct spiffs_file_s *fobj;
+  ssize_t nflushed;
   int16_t data_spndx;
   int16_t objndx_spndx;
   off_t fsize;
@@ -847,7 +864,11 @@ static off_t spiffs_seek(FAR struct file *filep, off_t offset, int whence)
 
   /* Get the new file offset */
 
-  spiffs_fflush_cache(fs, fobj);
+  nflushed = spiffs_fobj_flush(fs, fobj);
+  if (nflushed < 0)
+    {
+      ferr("ERROR: spiffs_fobj_flush() failed: %d\n", ret);
+    }
 
   fsize = fobj->size == SPIFFS_UNDEFINED_LEN ? 0 : fobj->size;
 
@@ -1045,7 +1066,8 @@ static int spiffs_sync(FAR struct file *filep)
   FAR struct inode *inode;
   FAR struct spiffs_s *fs;
   FAR struct spiffs_file_s *fobj;
-  int ret;
+  ssize_t nflushed;
+  int ret = OK;
 
   finfo("filep=%p\n", filep);
   DEBUGASSERT(filep->f_priv != NULL && filep->f_inode != NULL);
@@ -1068,7 +1090,12 @@ static int spiffs_sync(FAR struct file *filep)
 
   /* Flush all cached write data */
 
-  ret = spiffs_fflush_cache(fs, fobj);
+  nflushed = spiffs_fobj_flush(fs, fobj);
+  if (nflushed < 0)
+    {
+      ferr("ERROR: spiffs_fobj_flush() failed: %d\n", ret);
+      ret = (int)nflushed;
+    }
 
   spiffs_unlock_volume(fs);
   return spiffs_map_errno(ret);
@@ -1149,7 +1176,7 @@ static int spiffs_fstat(FAR const struct file *filep, FAR struct stat *buf)
 
   /* Flush the cache and perform the common stat() operation */
 
-  spiffs_fflush_cache(fs, fobj);
+  spiffs_fobj_flush(fs, fobj);
 
   ret = spiffs_stat_pgndx(fs, fobj->objhdr_pgndx, fobj->objid, buf);
   if (ret < 0)
@@ -1650,7 +1677,7 @@ static int spiffs_unlink(FAR struct inode *mountpt, FAR const char *relpath)
       fobj = (FAR struct spiffs_file_s *)kmm_zalloc(sizeof(struct spiffs_file_s));
       if (fobj == NULL)
         {
-          fwarn("WARNING: Failed to allocate fobjs\n");
+          fwarn("WARNING: Failed to allocate fobj\n");
           ret = -ENOMEM;
           goto errout_with_lock;
         }

fs/spiffs/src/spiffs_volume.c

@@ -209,7 +209,7 @@ int spiffs_find_fobj_byobjid(FAR struct spiffs_s *fs, int16_t objid,
 }
 
 /****************************************************************************
- * Name: spiffs_fflush_cache
+ * Name: spiffs_fobj_flush
  *
  * Description:
  *   Checks if there are any cached writes for the object ID associated with
@@ -225,8 +225,8 @@ int spiffs_find_fobj_byobjid(FAR struct spiffs_s *fs, int16_t objid,
  *
  ****************************************************************************/
 
-ssize_t spiffs_fflush_cache(FAR struct spiffs_s *fs,
+ssize_t spiffs_fobj_flush(FAR struct spiffs_s *fs,
-                            FAR struct spiffs_file_s *fobj)
+                          FAR struct spiffs_file_s *fobj)
 {
   ssize_t nwritten = 0;
 
@@ -382,7 +382,7 @@ ssize_t spiffs_fobj_read(FAR struct spiffs_s *fs,
       return 0;
     }
 
-  spiffs_fflush_cache(fs, fobj);
+  spiffs_fobj_flush(fs, fobj);
 
   /* Check for attempts to read beyond the end of the file */
 
@@ -444,10 +444,10 @@ void spiffs_fobj_free(FAR struct spiffs_s *fs,
 
   /* Flush any buffered write data */
 
-  ret = spiffs_fflush_cache(fs, fobj);
+  ret = spiffs_fobj_flush(fs, fobj);
   if (ret < 0)
     {
-      ferr("ERROR: spiffs_fflush_cache failed: %d\n", ret);
+      ferr("ERROR: spiffs_fobj_flush failed: %d\n", ret);
     }
 
   /* Remove the file object from the list of file objects in the volume