From 41c40fdd75222d95ff819656571a4ae4d61d571b Mon Sep 17 00:00:00 2001 From: patacongo Date: Sat, 31 May 2008 22:10:21 +0000 Subject: Fix several problems with accessing FAT filesystems git-svn-id: svn://svn.code.sf.net/p/nuttx/code/trunk@758 42af7a65-404d-4744-a932-0658087f49c3 --- nuttx/ChangeLog | 11 ++- nuttx/Documentation/NuttX.html | 11 ++- nuttx/fs/fs_fat32.c | 205 +++++++++++++++++++++++------------------ nuttx/fs/fs_fat32util.c | 9 +- 4 files changed, 133 insertions(+), 103 deletions(-) diff --git a/nuttx/ChangeLog b/nuttx/ChangeLog index 180762c8b..82230ff0b 100644 --- a/nuttx/ChangeLog +++ b/nuttx/ChangeLog @@ -353,11 +353,14 @@ * Add configuration for the ZiLOG z8f64200100kit development kit, Z8F6423 part. * Add configuration for the ZiLOG ez80f0910200kitg development kit, EZ80F091 part. * Correct critical list handling errors in task shutdown logic: One in timer - deletion logic (timer_delete.c) and one in stream logic (lib_init.c). + deletion logic (timer_delete.c) and one in stream logic (lib_init.c) reported + by kwonsk. 0.3.11 2008-xx-xx Gregory Nutt - * Add support for recursive mutexes. + * FEATURE: Add support for recursive mutexes. * BUGFIX: Eliminate a memory leak -- contained watchdog instance was not - being deleted with a POSIX timer was deleted. - * BUGFIX: Eliminate a deadlock condition in opendir(). + being deleted with a POSIX timer was deleted reported by kwonsk. + * BUGFIX: Eliminate a deadlock condition in opendir() reported by kwonsk. + * BUGFIX: Fix several FAT filesystem problems reported by kwonsk (Changes + not yet verified). diff --git a/nuttx/Documentation/NuttX.html b/nuttx/Documentation/NuttX.html index 3efaa0ebf..1ed7d6e62 100644 --- a/nuttx/Documentation/NuttX.html +++ b/nuttx/Documentation/NuttX.html @@ -979,7 +979,8 @@ nuttx-0.3.10 2008-05-15 Gregory Nutt <spudmonkey@racsa.co.cr> * Add configuration for the ZiLOG z8f64200100kit development kit, Z8F6423 part. * Add configuration for the ZiLOG ez80f0910200kitg development kit, EZ80F091 part. * Correct critical list handling errors in task shutdown logic: One in timer - deletion logic (timer_delete.c) and one in stream logic (lib_init.c). + deletion logic (timer_delete.c) and one in stream logic (lib_init.c) reported + by kwonsk. pascal-0.1.2 2008-02-10 Gregory Nutt @@ -1007,10 +1008,12 @@ buildroot-0.1.0 2007-03-09 <spudmonkey@racsa.co.cr> 
    
 nuttx-0.3.11 2008-xx-xx Gregory Nutt <spudmonkey@racsa.co.cr>
 
-	* Add support for recursive mutexes.
+	* FEATURE: Add support for recursive mutexes.
 	* BUGFIX: Eliminate a memory leak -- contained watchdog instance was not
-	  being deleted with a POSIX timer was deleted.
-	* BUGFIX: Eliminate a deadlock condition in opendir().
+	  being deleted with a POSIX timer was deleted reported by kwonsk.
+	* BUGFIX: Eliminate a deadlock condition in opendir() reported by kwonsk.
+	* BUGFIX: Fix several FAT filesystem problems reported by kwonsk (Changes
+	  not yet verified).
 
 pascal-0.1.3 2008-xx-xx Gregory Nutt <spudmonkey@racsa.co.cr>
 
diff --git a/nuttx/fs/fs_fat32.c b/nuttx/fs/fs_fat32.c
index 9d1b273f2..a54298bce 100644
--- a/nuttx/fs/fs_fat32.c
+++ b/nuttx/fs/fs_fat32.c
@@ -1,12 +1,13 @@
 /****************************************************************************
  * fs_fat32.c
  *
- *   Copyright (C) 2007 Gregory Nutt. All rights reserved.
+ *   Copyright (C) 2007, 2008 Gregory Nutt. All rights reserved.
  *   Author: Gregory Nutt 
  *
  * References:
  *   Microsoft FAT documentation
- *   FAT implementation 'Copyright (C) 2007, ChaN, all right reserved.'
+ *   Some good ideas were leveraged from the FAT implementation:
+ *     'Copyright (C) 2007, ChaN, all right reserved.'
  *     which has an unrestricted license.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -19,7 +20,7 @@
  *    notice, this list of conditions and the following disclaimer in
  *    the documentation and/or other materials provided with the
  *    distribution.
- * 3. Neither the name Gregory Nutt nor the names of its contributors may be
+ * 3. Neither the name NuttX nor the names of its contributors may be
  *    used to endorse or promote products derived from this software
  *    without specific prior written permission.
  *
@@ -262,7 +263,7 @@ static int fat_open(FAR struct file *filp, const char *relpath,
       /* Yes.. create the file */
 
       ret = fat_dircreate(fs, &dirinfo);
-      if ( ret < 0)
+      if (ret < 0)
         {
           goto errout_with_semaphore;
         }
@@ -312,7 +313,7 @@ static int fat_open(FAR struct file *filp, const char *relpath,
   /* File cluster/size info */
 
   ff->ff_startcluster     =
-      ((uint32)DIR_GETFSTCLUSTHI(dirinfo.fd_entry) << 16) |
+    ((uint32)DIR_GETFSTCLUSTHI(dirinfo.fd_entry) << 16) |
       DIR_GETFSTCLUSTLO(dirinfo.fd_entry);
 
   ff->ff_size             = DIR_GETFILESIZE(dirinfo.fd_entry);
@@ -344,10 +345,10 @@ static int fat_open(FAR struct file *filp, const char *relpath,
    * handling a lot simpler.
    */
 
- errout_with_struct:
+errout_with_struct:
   free(ff);
 
- errout_with_semaphore:
+errout_with_semaphore:
   fat_semgive(fs);
   return ret;
 }
@@ -444,7 +445,7 @@ static ssize_t fat_read(FAR struct file *filp, char *buffer, size_t buflen)
 
   if ((ff->ff_oflags & O_RDOK) == 0)
     {
-      ret= -EACCES;
+      ret = -EACCES;
       goto errout_with_semaphore;
     }
 
@@ -465,12 +466,14 @@ static ssize_t fat_read(FAR struct file *filp, char *buffer, size_t buflen)
    * error occurs.
    */
 
-  readsize = 0;
+  readsize   = 0;
+  readsector = ff->ff_currentsector;
   while (buflen > 0)
     {
       /* Get offset into the sector where we begin the read */
 
       int sectorindex = ff->ff_position & SEC_NDXMASK(fs);
+      bytesread = 0;
 
       /* Check if the current read stream happens to lie on a
        * sector boundary.
@@ -523,84 +526,86 @@ static ssize_t fat_read(FAR struct file *filp, char *buffer, size_t buflen)
               /* Setup to read the first sector from the new cluster */
 
               ff->ff_currentcluster   = cluster;
-              readsector              = fat_cluster2sector(fs, cluster);
               ff->ff_sectorsincluster = fs->fs_fatsecperclus;
+              readsector              = fat_cluster2sector(fs, cluster);
             }
+        }
 
-          /* Check if the user has provided a buffer large enough to
-           * hold one or more complete sectors.
+      /* Check if the user has provided a buffer large enough to
+       * hold one or more complete sectors.
+       */
+
+      nsectors = buflen / fs->fs_hwsectorsize;
+      if (nsectors > 0)
+        {
+          /* Read maximum contiguous sectors directly to the user's
+           * buffer without using our tiny read buffer.
+           *
+           * Limit the number of sectors that we read on this time
+           * through the loop to the remaining contiguous sectors
+           * in this cluster
            */
 
-          nsectors = buflen / fs->fs_hwsectorsize;
-          if (nsectors > 0)
+          if (nsectors > ff->ff_sectorsincluster)
             {
-              /* Read maximum contiguous sectors directly to the user's
-               * buffer without using our tiny read buffer.
-               *
-               * Limit the number of sectors that we read on this time
-               * through the loop to the remaining contiguous sectors
-               * in this cluster
-               */
-
-              if (nsectors > ff->ff_sectorsincluster)
-                {
-                  nsectors = ff->ff_sectorsincluster;
-                }
-
-              /* We are not sure of the state of the file buffer so
-               * the safest thing to do is just invalidate it
-               */
+              nsectors = ff->ff_sectorsincluster;
+            }
 
-              (void)fat_ffcacheinvalidate(fs, ff);
+          /* We are not sure of the state of the file buffer so
+           * the safest thing to do is just invalidate it
+           */
 
-              /* Read all of the sectors directory into user memory */
+          (void)fat_ffcacheinvalidate(fs, ff);
 
-              ret = fat_hwread(fs, userbuffer, readsector, nsectors);
-              if (ret < 0)
-                {
-                  goto errout_with_semaphore;
-                }
+          /* Read all of the sectors directory into user memory */
 
-              ff->ff_sectorsincluster -= nsectors - 1;
-              bytesread                = nsectors * fs->fs_hwsectorsize;
-            }
-          else
+          ret = fat_hwread(fs, userbuffer, readsector, nsectors);
+          if (ret < 0)
             {
-              /* We are reading a partial sector.  First, read the whole sector
-               * into the file data buffer.  This is a caching buffer so if
-               * it is already there then all is well.
-               */
+              goto errout_with_semaphore;
+            }
 
-              ret = fat_ffcacheread(fs, ff, readsector);
-              if (ret < 0)
-                {
-                  goto errout_with_semaphore;
-                }
+          ff->ff_sectorsincluster -= nsectors - 1;
+          ff->ff_currentsector     = readsector + nsectors - 1;
+          bytesread                = nsectors * fs->fs_hwsectorsize;
+        }
+      else
+        {
+          /* We are reading a partial sector.  First, read the whole sector
+           * into the file data buffer.  This is a caching buffer so if
+           * it is already there then all is well.
+           */
 
-              /* Copy the partial sector into the user buffer */
+          ret = fat_ffcacheread(fs, ff, readsector);
+          if (ret < 0)
+            {
+              goto errout_with_semaphore;
+            }
 
-              bytesread = fs->fs_hwsectorsize - sectorindex;
-              if (bytesread > buflen)
-                {
-                  bytesread = buflen;
-                }
+          /* Copy the partial sector into the user buffer */
 
-              memcpy(userbuffer, &ff->ff_buffer[sectorindex], bytesread);
+          bytesread = fs->fs_hwsectorsize - sectorindex;
+          if (bytesread > buflen)
+            {
+              bytesread = buflen;
             }
 
-          /* Set up for the next sector read */
-
-          userbuffer      += bytesread;
-          ff->ff_position += bytesread;
-          readsize        += bytesread;
-          buflen          -= bytesread;
+          memcpy(userbuffer, &ff->ff_buffer[sectorindex], bytesread);
+          ff->ff_currentsector = readsector;
         }
+
+      /* Set up for the next sector read */
+
+      userbuffer      += bytesread;
+      ff->ff_position += bytesread;
+      readsize        += bytesread;
+      buflen          -= bytesread;
     }
 
   fat_semgive(fs);
   return readsize;
 
- errout_with_semaphore:
+errout_with_semaphore:
   fat_semgive(fs);
   return ret;
 }
@@ -648,7 +653,7 @@ static ssize_t fat_write(FAR struct file *filp, const char *buffer,
 
   if ((ff->ff_oflags & O_WROK) == 0)
     {
-      ret= -EACCES;
+      ret = -EACCES;
       goto errout_with_semaphore;
     }
 
@@ -665,6 +670,7 @@ static ssize_t fat_write(FAR struct file *filp, const char *buffer,
    */
 
   byteswritten = 0;
+  writesector = ff->ff_currentsector;
   while (buflen > 0)
     {
       /* Get offset into the sector where we begin the read */
@@ -744,8 +750,8 @@ static ssize_t fat_write(FAR struct file *filp, const char *buffer,
               /* Setup to write the first sector from the new cluster */
 
               ff->ff_currentcluster   = cluster;
-              writesector             = fat_cluster2sector(fs, cluster);
               ff->ff_sectorsincluster = fs->fs_fatsecperclus;
+              writesector             = fat_cluster2sector(fs, cluster);
             }
         }
 
@@ -792,6 +798,7 @@ static ssize_t fat_write(FAR struct file *filp, const char *buffer,
             }
 
           ff->ff_sectorsincluster -= nsectors - 1;
+          ff->ff_currentsector     = writesector + nsectors - 1;
           writesize                = nsectors * fs->fs_hwsectorsize;
           ff->ff_bflags           |= FFBUFF_MODIFIED;
         }
@@ -843,7 +850,7 @@ static ssize_t fat_write(FAR struct file *filp, const char *buffer,
   fat_semgive(fs);
   return byteswritten;
 
- errout_with_semaphore:
+errout_with_semaphore:
   fat_semgive(fs);
   return ret;
 }
@@ -1074,7 +1081,7 @@ static off_t fat_seek(FAR struct file *filp, off_t offset, int whence)
   fat_semgive(fs);
   return OK;
 
- errout_with_semaphore:
+errout_with_semaphore:
   fat_semgive(fs);
   return ret;
 }
@@ -1210,7 +1217,7 @@ static int fat_sync(FAR struct file *filp)
       ret          = fat_updatefsinfo(fs);
     }
 
- errout_with_semaphore:
+errout_with_semaphore:
   fat_semgive(fs);
   return ret;
 }
@@ -1257,12 +1264,14 @@ static int fat_opendir(struct inode *mountpt, const char *relpath, struct intern
 
   if (dirinfo.fd_entry == NULL)
     {
-      /* Handler the FAT12/16 root directory */
+      /* Handle the FAT12/16/32 root directory using the values setup by
+       * fat_finddirentry() above.
+       */
 
-      dir->u.fat.fd_startcluster = 0;
-      dir->u.fat.fd_currcluster  = 0;
-      dir->u.fat.fd_currsector   = fs->fs_rootbase;
-      dir->u.fat.fd_index        = 2;
+      dir->u.fat.fd_startcluster = dirinfo.dir.fd_startcluster;
+      dir->u.fat.fd_currcluster  = dirinfo.dir.fd_currcluster;
+      dir->u.fat.fd_currsector   = dirinfo.dir.fd_currsector;
+      dir->u.fat.fd_index        = dirinfo.dir.fd_index;
     }
 
   /* This is not the root directory.  Verify that it is some kind of directory */
@@ -1307,7 +1316,7 @@ static int fat_readdir(struct inode *mountpt, struct internal_dir_s *dir)
   ubyte                  *direntry;
   ubyte                   ch;
   ubyte                   attribute;
-  int                     ret;
+  int                     ret = OK;
 
   /* Sanity checks */
 
@@ -1332,7 +1341,7 @@ static int fat_readdir(struct inode *mountpt, struct internal_dir_s *dir)
   while (dir->u.fat.fd_currsector && dir->fd_dir.d_name[0] == '\0')
     {
       ret = fat_fscacheread(fs, dir->u.fat.fd_currsector);
-      if ( ret < 0)
+      if (ret < 0)
         {
           goto errout_with_semaphore;
         }
@@ -1380,7 +1389,8 @@ static int fat_readdir(struct inode *mountpt, struct internal_dir_s *dir)
 
       if (fat_nextdirentry(fs, &dir->u.fat) != OK)
         {
-          dir->u.fat.fd_currsector = 0;
+          ret = -ENOENT;
+          goto errout_with_semaphore;
         }
     }
 
@@ -1389,7 +1399,7 @@ static int fat_readdir(struct inode *mountpt, struct internal_dir_s *dir)
 
 errout_with_semaphore:
   fat_semgive(fs);
-  return ERROR;
+  return ret;
 }
 
 /****************************************************************************
@@ -1423,13 +1433,23 @@ static int fat_rewinddir(struct inode *mountpt, struct internal_dir_s *dir)
 
   /* Check if this is the root directory */
 
-  if (dir->u.fat.fd_startcluster == 0)
+  if (fs->fs_type != FSTYPE_FAT32 &&
+      dir->u.fat.fd_startcluster == 0)
     {
-      /* Handler the FAT12/16 root directory */
+      /* Handle the FAT12/16 root directory */
 
       dir->u.fat.fd_currcluster  = 0;
       dir->u.fat.fd_currsector   = fs->fs_rootbase;
-      dir->u.fat.fd_index        = 2;
+      dir->u.fat.fd_index        = 0;
+    }
+  else if (fs->fs_type == FSTYPE_FAT32 &&
+           dir->u.fat.fd_startcluster == fs->fs_rootbase)
+    {
+      /* Handle the FAT32 root directory */
+
+      dir->u.fat.fd_currcluster = dir->u.fat.fd_startcluster;
+      dir->u.fat.fd_currsector  = fat_cluster2sector(fs, fs->fs_rootbase);
+      dir->u.fat.fd_index       = 0;
     }
 
   /* This is not the root directory */
@@ -1473,7 +1493,7 @@ static int fat_bind(FAR struct inode *blkdriver, const void *data,
       return -ENODEV;
     }
 
-  if ( blkdriver->u.i_bops->open &&
+  if (blkdriver->u.i_bops->open &&
        blkdriver->u.i_bops->open(blkdriver) != OK)
     {
       return -ENODEV;
@@ -1482,7 +1502,7 @@ static int fat_bind(FAR struct inode *blkdriver, const void *data,
   /* Create an instance of the mountpt state structure */
 
   fs = (struct fat_mountpt_s *)zalloc(sizeof(struct fat_mountpt_s));
-  if ( !fs )
+  if (!fs)
     {
       return -ENOMEM;
     }
@@ -1500,7 +1520,7 @@ static int fat_bind(FAR struct inode *blkdriver, const void *data,
    */
 
   ret = fat_mount(fs, TRUE);
-  if ( ret != 0 )
+  if (ret != 0)
     {
       sem_destroy(&fs->fs_sem);
       free(fs);
@@ -1525,7 +1545,7 @@ static int fat_unbind(void *handle, FAR struct inode **blkdriver)
   struct fat_mountpt_s *fs = (struct fat_mountpt_s*)handle;
   int ret;
 
-  if ( !fs )
+  if (!fs)
     {
       return -EINVAL;
     }
@@ -1881,7 +1901,7 @@ static int fat_mkdir(struct inode *mountpt, const char *relpath, mode_t mode)
   fat_semgive(fs);
   return OK;
 
- errout_with_semaphore:
+errout_with_semaphore:
   fat_semgive(fs);
   return ret;
 }
@@ -2061,7 +2081,7 @@ int fat_rename(struct inode *mountpt, const char *oldrelpath,
   fat_semgive(fs);
   return OK;
 
- errout_with_semaphore:
+errout_with_semaphore:
   fat_semgive(fs);
   return ret;
 }
@@ -2111,9 +2131,13 @@ static int fat_stat(struct inode *mountpt, const char *relpath, struct stat *buf
       goto errout_with_semaphore;
     }
 
-  if (! dirinfo.fd_entry)
+  memset(buf, 0, sizeof(struct stat));
+  if (!dirinfo.fd_entry)
     {
-      ret = -ENOENT;
+      /* It's directory name of mount point */
+
+      buf->st_mode = S_IFDIR|S_IROTH|S_IRGRP|S_IRUSR|S_IWOTH|S_IWGRP|S_IWUSR;
+      ret = OK;
       goto errout_with_semaphore;
     }
 
@@ -2130,7 +2154,6 @@ static int fat_stat(struct inode *mountpt, const char *relpath, struct stat *buf
    * by everyone but may be writeable by no-one.
    */
 
-  memset(buf, 0, sizeof(struct stat));
   buf->st_mode = S_IROTH|S_IRGRP|S_IRUSR;
   if ((attribute & FATATTR_READONLY) == 0)
     {
@@ -2176,7 +2199,7 @@ static int fat_stat(struct inode *mountpt, const char *relpath, struct stat *buf
 
   ret = OK;
 
- errout_with_semaphore:
+errout_with_semaphore:
   fat_semgive(fs);
   return ret;
 }
diff --git a/nuttx/fs/fs_fat32util.c b/nuttx/fs/fs_fat32util.c
index 5931d76d0..48d1a6e19 100644
--- a/nuttx/fs/fs_fat32util.c
+++ b/nuttx/fs/fs_fat32util.c
@@ -6,7 +6,8 @@
  *
  * References:
  *   Microsoft FAT documentation
- *   FAT implementation 'Copyright (C) 2007, ChaN, all right reserved.'
+ *   Some good ideas were leveraged from the FAT implementation:
+ *     'Copyright (C) 2007, ChaN, all right reserved.'
  *     which has an unrestricted license.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -1340,7 +1341,7 @@ int fat_nextdirentry(struct fat_mountpt_s *fs, struct fs_fatdir_s *dir)
    * been examined.
    */
 
-  if (ndx >= DIRSEC_NDIRS(fs))
+  if ((ndx & (DIRSEC_NDIRS(fs)-1)) == 0)
     {
       /* Yes, then we will have to read the next sector */
 
@@ -1352,7 +1353,7 @@ int fat_nextdirentry(struct fat_mountpt_s *fs, struct fs_fatdir_s *dir)
 
       if (!dir->fd_currcluster)
         {
-          /* For FAT12/13, the boot record tells us number of 32-bit directories
+          /* For FAT12/16, the boot record tells us number of 32-bit directories
            * that are contained in the root directory.  This should correspond to
            * an even number of sectors.
            */
@@ -1381,7 +1382,7 @@ int fat_nextdirentry(struct fat_mountpt_s *fs, struct fs_fatdir_s *dir)
            * has been examined.
            */
 
-          if (sector >= fs->fs_fatsecperclus)
+          if ((sector & (fs->fs_fatsecperclus-1)) == 0)
             {
               /* Get next cluster */
 
-- 
cgit v1.2.3