mkfatfs: Fix an error in logic that determines if FAT16 is possible

ChangeLog

@@ -6162,5 +6162,6 @@
 	* drivers/mtd/README.txt:  New README file (2013-12-04).
 	* arch/arm/src/lm/lm_start.c:  Don't initialize .data if not running
 	  from FLASH (2013-12-05).
-
+	* fs/fat/fs_configfat.c: Fix a typo in the FAT16 formatting logic.
+	  Was this ever able to format a FAT16 disk? (2013-12-05).
 

fs/fat/fs_configfat.c

@@ -1,7 +1,7 @@
 /****************************************************************************
  * fs/fat/fs_configfat.c
  *
- *   Copyright (C) 2008-2009 Gregory Nutt. All rights reserved.
+ *   Copyright (C) 2008-2009, 2013 Gregory Nutt. All rights reserved.
  *   Author: Gregory Nutt <gnutt@nuttx.org>
  *
  * Redistribution and use in source and binary forms, with or without
@@ -132,6 +132,7 @@ static uint8_t g_bootcodeblob[] =
  *  >0: The size of one FAT in sectors.
  *
  ****************************************************************************/
+
 static inline uint32_t
 mkfatfs_nfatsect12(FAR struct fat_format_s *fmt, FAR struct fat_var_s *var,
                    uint32_t navailsects)
@@ -209,6 +210,7 @@ mkfatfs_nfatsect12(FAR struct fat_format_s *fmt, FAR struct fat_var_s *var,
  *    The size of one FAT in sectors.
  *
  ****************************************************************************/
+
 static inline uint32_t
 mkfatfs_nfatsect16(FAR struct fat_format_s *fmt, FAR struct fat_var_s *var,
                    uint32_t navailsects)
@@ -253,6 +255,7 @@ mkfatfs_nfatsect16(FAR struct fat_format_s *fmt, FAR struct fat_var_s *var,
       denom = fmt->ff_nfats + (var->fv_sectorsize << (fmt->ff_clustshift - 1));
       numer = navailsects + (1 << (fmt->ff_clustshift + 1));
     }
+
   return (uint32_t)((numer + denom - 1) / denom);
 }
 
@@ -275,6 +278,7 @@ mkfatfs_nfatsect16(FAR struct fat_format_s *fmt, FAR struct fat_var_s *var,
  *   The size of one FAT in sectors.
  *
  ****************************************************************************/
+
 static inline uint32_t
 mkfatfs_nfatsect32(FAR struct fat_format_s *fmt, FAR struct fat_var_s *var,
                    uint32_t navailsects)
@@ -324,6 +328,7 @@ mkfatfs_nfatsect32(FAR struct fat_format_s *fmt, FAR struct fat_var_s *var,
       denom = fmt->ff_nfats + (var->fv_sectorsize << (fmt->ff_clustshift - 2));
       numer = navailsects + (1 << (fmt->ff_clustshift + 1)) + (1 << fmt->ff_clustshift);
     }
+
   return (uint32_t)((numer + denom - 1) / denom);
 }
 
@@ -344,6 +349,7 @@ mkfatfs_nfatsect32(FAR struct fat_format_s *fmt, FAR struct fat_var_s *var,
  *   size is the returned value.
  *
  ****************************************************************************/
+
 static inline uint8_t
 mkfatfs_clustersearchlimits(FAR struct fat_format_s *fmt, FAR struct fat_var_s *var)
 {
@@ -402,6 +408,7 @@ mkfatfs_clustersearchlimits(FAR struct fat_format_s *fmt, FAR struct fat_var_s *
 
       mxclustshift = fmt->ff_clustshift;
     }
+
   return mxclustshift;
 }
 
@@ -416,13 +423,14 @@ mkfatfs_clustersearchlimits(FAR struct fat_format_s *fmt, FAR struct fat_var_s *
  *   fmt - Caller specified format parameters
  *   var - Other format parameters that are not caller specifiable. (Most
  *     set by mkfatfs_configfatfs()).
- *   fatconfig - FAT12-specific configuration
+ *   config - FAT12-specific configuration
  *
  * Return:
  *    Zero on success configuration of a FAT12 file system; negated errno
  *    on failure
  *
  ****************************************************************************/
+
 static inline int
 mkfatfs_tryfat12(FAR struct fat_format_s *fmt, FAR struct fat_var_s *var,
                  FAR struct fat_config_s *config)
@@ -455,6 +463,7 @@ mkfatfs_tryfat12(FAR struct fat_format_s *fmt, FAR struct fat_var_s *var,
         {
           maxnclusters = FAT_MAXCLUST12;
         }
+
       fvdbg("nfatsects=%u nclusters=%u (max=%u)\n",
             config->fc_nfatsects, config->fc_nclusters, maxnclusters);
 
@@ -464,10 +473,13 @@ mkfatfs_tryfat12(FAR struct fat_format_s *fmt, FAR struct fat_var_s *var,
 
       if (config->fc_nclusters + 2 > maxnclusters)
         {
-          fvdbg("Too many clusters for FAT12\n");
+          fdbg("Too many clusters for FAT12: %d > %d\n",
+               config->fc_nclusters, maxnclusters - 2);
+
           return -ENFILE;
         }
     }
+
   return 0;
 }
 
@@ -482,13 +494,14 @@ mkfatfs_tryfat12(FAR struct fat_format_s *fmt, FAR struct fat_var_s *var,
  *   fmt - Caller specified format parameters
  *   var - Other format parameters that are not caller specifiable. (Most
  *     set by mkfatfs_configfatfs()).
- *   fatconfig - FAT16-specific configuration
+ *   config - FAT16-specific configuration
  *
  * Return:
  *    Zero on success configuration of a FAT16 file system; negated errno
  *    on failure
  *
  ****************************************************************************/
+
 static inline int
 mkfatfs_tryfat16(FAR struct fat_format_s *fmt, FAR struct fat_var_s *var,
                  FAR struct fat_config_s *config)
@@ -516,13 +529,15 @@ mkfatfs_tryfat16(FAR struct fat_format_s *fmt, FAR struct fat_var_s *var,
        *   maxnclusters = nfatsects * sectorsize / 2 - 2
        */
 
-      maxnclusters = config->fc_nfatsects << (var->fv_sectorsize - 1);
+      maxnclusters = config->fc_nfatsects << (var->fv_sectshift - 1);
       if (maxnclusters > FAT_MAXCLUST16)
         {
           maxnclusters = FAT_MAXCLUST16;
         }
+
       fvdbg("nfatsects=%u nclusters=%u (min=%u max=%u)\n",
-            config->fc_nfatsects, config->fc_nclusters, FAT_MINCLUST16, maxnclusters);
+            config->fc_nfatsects, config->fc_nclusters, FAT_MINCLUST16,
+            maxnclusters);
 
       /* Check if this number of clusters would overflow the maximum for
        * FAT16 (remembering that two FAT cluster slots are reserved).
@@ -535,10 +550,13 @@ mkfatfs_tryfat16(FAR struct fat_format_s *fmt, FAR struct fat_var_s *var,
       if ((config->fc_nclusters + 2 > maxnclusters) ||
           (config->fc_nclusters < FAT_MINCLUST16))
         {
-          fvdbg("Too few or too many clusters for FAT16\n");
+          fdbg("Too few or too many clusters for FAT16: %d < %d < %d\n",
+               FAT_MINCLUST16, config->fc_nclusters, maxnclusters - 2);
+
           return -ENFILE;
         }
     }
+
   return 0;
 }
 
@@ -553,13 +571,14 @@ mkfatfs_tryfat16(FAR struct fat_format_s *fmt, FAR struct fat_var_s *var,
  *   fmt - Caller specified format parameters
  *   var - Other format parameters that are not caller specifiable. (Most
  *     set by mkfatfs_configfatfs()).
- *   fatconfig - FAT32-specific configuration
+ *   config - FAT32-specific configuration
  *
  * Return:
  *    Zero on success configuration of a FAT32 file system; negated errno
  *    on failure
  *
  ****************************************************************************/
+
 static inline int
 mkfatfs_tryfat32(FAR struct fat_format_s *fmt, FAR struct fat_var_s *var,
                  FAR struct fat_config_s *config)
@@ -592,6 +611,7 @@ mkfatfs_tryfat32(FAR struct fat_format_s *fmt, FAR struct fat_var_s *var,
         {
           maxnclusters = FAT_MAXCLUST32;
         }
+
       fvdbg("nfatsects=%u nclusters=%u (max=%u)\n",
             config->fc_nfatsects, config->fc_nclusters, maxnclusters);
 
@@ -600,12 +620,15 @@ mkfatfs_tryfat32(FAR struct fat_format_s *fmt, FAR struct fat_var_s *var,
        */
 
       if ((config->fc_nclusters + 3 > maxnclusters) ||
-          (config->fc_nclusters < FAT_MINCLUST32 && fmt->ff_fattype != 32))
+          (config->fc_nclusters < FAT_MINCLUST32))
         {
-          fvdbg("Too few or too many clusters for FAT32\n");
+          fdbg("Too few or too many clusters for FAT32: %d < %d < %d\n",
+               FAT_MINCLUST32, config->fc_nclusters, maxnclusters - 3);
+
           return -ENFILE;
         }
     }
+
   return 0;
 }
 
@@ -631,7 +654,8 @@ mkfatfs_selectfat(int fattype, FAR struct fat_format_s *fmt,
 {
   /* Return the appropriate information about the selected file system. */
 
-  fdbg("Selected FAT%d\n", fattype);
+  fvdbg("Selected FAT%d\n", fattype);
+
   var->fv_fattype      = fattype;
   var->fv_nclusters    = config->fc_nclusters;
   var->fv_nfatsects    = config->fc_nfatsects;
@@ -672,7 +696,7 @@ mkfatfs_clustersearch(FAR struct fat_format_s *fmt, FAR struct fat_var_s *var)
 
       if (fmt->ff_rsvdseccount < 2)
         {
-          fvdbg("At least 2 reserved sectors needed by FAT32\n");
+          fdbg("At least 2 reserved sectors needed by FAT32\n");
           fatconfig32.fc_rsvdseccount = 2;
         }
       else
@@ -721,7 +745,8 @@ mkfatfs_clustersearch(FAR struct fat_format_s *fmt, FAR struct fat_var_s *var)
 
   do
     {
-      fvdbg("Configuring with %d sectors/cluster...\n", 1 << fmt->ff_clustshift);
+      fvdbg("Configuring with %d sectors/cluster...\n",
+            1 << fmt->ff_clustshift);
 
       /* Check if FAT12 has not been excluded */
 
@@ -731,13 +756,13 @@ mkfatfs_clustersearch(FAR struct fat_format_s *fmt, FAR struct fat_var_s *var)
 
           if (mkfatfs_tryfat12(fmt, var, &fatconfig12) != 0)
             {
-                {
+              fdbg("Cannot format FAT12 at %u sectors/cluster\n",
-                  fvdbg("Cannot format FAT12 at %u sectors/cluster\n", 1 << fmt->ff_clustshift);
+                   1 << fmt->ff_clustshift);
+
               fatconfig12.fc_nfatsects = 0;
               fatconfig12.fc_nclusters = 0;
             }
         }
-        }
 
       /* Check if FAT16 has not been excluded */
 
@@ -747,13 +772,13 @@ mkfatfs_clustersearch(FAR struct fat_format_s *fmt, FAR struct fat_var_s *var)
 
           if (mkfatfs_tryfat16(fmt, var, &fatconfig16) != 0)
             {
-                {
+              fdbg("Cannot format FAT16 at %u sectors/cluster\n",
-                  fvdbg("Cannot format FAT16 at %u sectors/cluster\n", 1 << fmt->ff_clustshift);
+                   1 << fmt->ff_clustshift);
+
               fatconfig16.fc_nfatsects = 0;
               fatconfig16.fc_nclusters = 0;
             }
         }
-        }
 
       /* If either FAT12 or 16 was configured at this sector/cluster setting,
        * then finish the configuration and break out now
@@ -778,12 +803,19 @@ mkfatfs_clustersearch(FAR struct fat_format_s *fmt, FAR struct fat_var_s *var)
 
               mkfatfs_selectfat(12, fmt, var, &fatconfig12);
             }
+
           return OK;
         }
 
+#if 0
       /* Check if FAT32 has not been excluded */
 
       if (var->fv_fattype == 0 || var->fv_fattype == 32)
+#else
+      /* FAT32 must be explicitly requested */
+
+      if (var->fv_fattype == 32)
+#endif
         {
           /* The number of data sectors available (includes the fat itself)
            * This value is a constant with respect to cluster sizefor FAT12/16, but not FAT32
@@ -796,12 +828,12 @@ mkfatfs_clustersearch(FAR struct fat_format_s *fmt, FAR struct fat_var_s *var)
 
           if (mkfatfs_tryfat32(fmt, var, &fatconfig32) != 0)
             {
-                {
+              fdbg("Cannot format FAT32 at %u sectors/cluster\n",
-                  fvdbg("Cannot format FAT32 at %u sectors/cluster\n", 1 << fmt->ff_clustshift);
+                   1 << fmt->ff_clustshift);
+
               fatconfig32.fc_nfatsects = 0;
               fatconfig32.fc_nclusters = 0;
             }
-            }
           else
             {
               /* Select FAT32 if we have not already done so */
@@ -816,6 +848,7 @@ mkfatfs_clustersearch(FAR struct fat_format_s *fmt, FAR struct fat_var_s *var)
       fmt->ff_clustshift++;
     }
   while (fmt->ff_clustshift <= mxclustshift);
+
   return -ENFILE;
 }
 
@@ -839,6 +872,7 @@ mkfatfs_clustersearch(FAR struct fat_format_s *fmt, FAR struct fat_var_s *var)
  *    Zero on success; negated errno on failure
  *
  ****************************************************************************/
+
 int mkfatfs_configfatfs(FAR struct fat_format_s *fmt,
                         FAR struct fat_var_s *var)
 {
@@ -863,7 +897,7 @@ int mkfatfs_configfatfs(FAR struct fat_format_s *fmt,
   ret = mkfatfs_clustersearch(fmt, var);
   if (ret < 0)
     {
-       fdbg("Failed to set cluster size\n");
+       fdbg("ERROR: Failed to set cluster size\n");
        return ret;
     }
 
@@ -947,10 +981,12 @@ int mkfatfs_configfatfs(FAR struct fat_format_s *fmt,
   fdbg("Sectors per cluster:  %d sectors\n",  1 << fmt->ff_clustshift);
   fdbg("FS size:              %d sectors\n",  var->fv_nfatsects);
   fdbg("                      %d clusters\n", var->fv_nclusters);
+
   if (var->fv_fattype != 32)
     {
        fdbg("Root directory slots: %d\n", fmt->ff_rootdirentries);
     }
+
   fdbg("Volume ID:            %08x\n", fmt->ff_volumeid);
   fdbg("Volume Label:         \"%c%c%c%c%c%c%c%c%c%c%c\"\n",
     fmt->ff_volumelabel[0], fmt->ff_volumelabel[1], fmt->ff_volumelabel[2],
@@ -960,4 +996,3 @@ int mkfatfs_configfatfs(FAR struct fat_format_s *fmt,
 #endif
   return OK;
 }
-

fs/fat/fs_fat32.h

@@ -316,7 +316,8 @@
 
 /* FAT32: M$ reserves the MS 4 bits of a FAT32 FAT entry so only 18 bits are
  * available.  For M$, the calculation is ((1 << 28) - 19). (The uint32_t cast
- * is needed for architectures where int is only 16 bits).
+ * is needed for architectures where int is only 16 bits).  M$ also claims
+ * that the minimum size is 65,527.
  */
 
 #define FAT_MINCLUST32  65524

fs/fat/fs_fat32util.c

@@ -149,7 +149,7 @@ static int fat_checkbootrecord(struct fat_mountpt_s *fs)
   /* Verify the FAT32 file system type. The determination of the file
    * system type is based on the number of clusters on the volume:  FAT12
    * volume has <= FAT_MAXCLUST12 (4084) clusters, a FAT16 volume has <=
-   * FAT_MINCLUST16 (Microsoft says < 65,525) clusters, and any larger
+   * FAT_MAXCLUST16 (Microsoft says < 65,525) clusters, and any larger
    * is FAT32.
    *
    * Get the number of 32-bit directory entries in root directory (zero
@@ -568,7 +568,7 @@ int fat_mount(struct fat_mountpt_s *fs, bool writeable)
   ret = fat_checkbootrecord(fs);
   if (ret != OK)
     {
-      /* The contents of sector 0 is not a boot record.  It could be a
+      /* The contents of sector 0 is not a boot record.  It could be a DOS
        * partition, however.  Assume it is a partition and get the offset
        * into the partition table.  This table is at offset MBR_TABLE and is
        * indexed by 16x the partition number.
@@ -603,7 +603,9 @@ int fat_mount(struct fat_mountpt_s *fs, bool writeable)
             {
               /* Failed to read the sector */
 
-              goto errout_with_buffer;
+              fdbg("ERROR: Failed to read sector %ld: %d\n",
+                   (long)fs->fs_fatbase, ret);
+              continue;
             }
 
           /* Check if this is a boot record */
@@ -623,6 +625,7 @@ int fat_mount(struct fat_mountpt_s *fs, bool writeable)
           ret = fat_hwread(fs, fs->fs_buffer, 0, 1);
           if (ret < 0)
             {
+              fdbg("ERROR: Failed to re-read sector 0: %d\n", ret);
               goto errout_with_buffer;
             }
         }
@@ -1866,5 +1869,3 @@ int fat_currentsector(struct fat_mountpt_s *fs, struct fat_file_s *ff,
 
   return -ENOSPC;
 }
-
-

fs/fat/fs_mkfatfs.c

@@ -86,13 +86,13 @@ static inline int mkfatfs_getgeometry(FAR struct fat_format_s *fmt,
   ret = DEV_GEOMETRY(geometry);
   if (ret < 0)
     {
-      fdbg("geometry() returned %d\n", ret);
+      fdbg("ERROR: geometry() returned %d\n", ret);
       return ret;
     }
 
   if (!geometry.geo_available || !geometry.geo_writeenabled)
     {
-      fdbg("Media is not available\n", ret);
+      fdbg("ERROR: Media is not available\n", ret);
       return -ENODEV;
     }
 
@@ -104,8 +104,9 @@ static inline int mkfatfs_getgeometry(FAR struct fat_format_s *fmt,
     {
       if (fmt->ff_nsectors > geometry.geo_nsectors)
         {
-          fdbg("User maxblocks (%d) exceeds blocks on device (%d)\n",
+          fdbg("ERROR: User maxblocks (%d) exceeds blocks on device (%d)\n",
                fmt->ff_nsectors, geometry.geo_nsectors);
+
           return -EINVAL;
         }
     }
@@ -138,9 +139,10 @@ static inline int mkfatfs_getgeometry(FAR struct fat_format_s *fmt,
         break;
 
       default:
-        fdbg("Unsupported sector size: %d\n", var->fv_sectorsize);
+        fdbg("ERROR: Unsupported sector size: %d\n", var->fv_sectorsize);
         return -EPERM;
     }
+
   return 0;
 }
 
@@ -152,7 +154,9 @@ static inline int mkfatfs_getgeometry(FAR struct fat_format_s *fmt,
  * Name: mkfatfs
  *
  * Description:
- *   Make a FAT file system image on the specified block device
+ *   Make a FAT file system image on the specified block device.  This
+ *   function can automatically format a FAT12 or FAT16 file system.  By
+ *   tradition, FAT32 will only be selected is explicitly requested.
  *
  * Inputs:
  *   pathname - the full path to a registered block driver
@@ -173,6 +177,7 @@ static inline int mkfatfs_getgeometry(FAR struct fat_format_s *fmt,
  *     device is indeterminate (but likely not good).
  *
  ****************************************************************************/
+
 int mkfatfs(FAR const char *pathname, FAR struct fat_format_s *fmt)
 {
   struct fat_var_s var;
@@ -191,14 +196,14 @@ int mkfatfs(FAR const char *pathname, FAR struct fat_format_s *fmt)
 #ifdef CONFIG_DEBUG
   if (!pathname)
     {
-      fdbg("No block driver path\n");
+      fdbg("ERROR: No block driver path\n");
       ret = -EINVAL;
       goto errout;
     }
 
   if (fmt->ff_nfats < 1 || fmt->ff_nfats > 4)
     {
-      fdbg("Invalid number of fats: %d\n", fmt->ff_nfats);
+      fdbg("ERROR: Invalid number of fats: %d\n", fmt->ff_nfats);
       ret = -EINVAL;
       goto errout;
     }
@@ -206,35 +211,48 @@ int mkfatfs(FAR const char *pathname, FAR struct fat_format_s *fmt)
   if (fmt->ff_fattype != 0  && fmt->ff_fattype != 12 &&
       fmt->ff_fattype != 16 && fmt->ff_fattype != 32)
     {
-      fdbg("Invalid FAT size: %d\n", fmt->ff_fattype);
+      fdbg("ERROR: Invalid FAT size: %d\n", fmt->ff_fattype);
       ret = -EINVAL;
       goto errout;
     }
 #endif
+
+  /* 0 will auto-selected by FAT12 and FAT16 (only).  Otherwise,
+   * fv_fattype will specify the exact format to use.
+   */
+
   var.fv_fattype = fmt->ff_fattype;
 
   /* The valid range off ff_clustshift is {0,1,..7} corresponding to
    * cluster sizes of {1,2,..128} sectors.  The special value of 0xff
    * means that we should autoselect the cluster sizel.
    */
+
 #ifdef CONFIG_DEBUG
   if (fmt->ff_clustshift > 7 && fmt->ff_clustshift != 0xff)
     {
-      fdbg("Invalid cluster shift value: %d\n", fmt->ff_clustshift);
+      fdbg("ERROR: Invalid cluster shift value: %d\n", fmt->ff_clustshift);
+
       ret = -EINVAL;
       goto errout;
     }
 
-   if (fmt->ff_rootdirentries != 0 && (fmt->ff_rootdirentries < 16 || fmt->ff_rootdirentries > 32767))
+   if (fmt->ff_rootdirentries != 0 &&
+       (fmt->ff_rootdirentries < 16 || fmt->ff_rootdirentries > 32767))
     {
-      fdbg("Invalid number of root dir entries: %d\n", fmt->ff_rootdirentries);
+      fdbg("ERROR: Invalid number of root dir entries: %d\n",
+           fmt->ff_rootdirentries);
+
       ret = -EINVAL;
       goto errout;
     }
 
-   if (fmt->ff_rsvdseccount != 0 && (fmt->ff_rsvdseccount < 1 || fmt->ff_rsvdseccount > 32767))
+   if (fmt->ff_rsvdseccount != 0 && (fmt->ff_rsvdseccount < 1 ||
+       fmt->ff_rsvdseccount > 32767))
     {
-      fdbg("Invalid number of reserved sectors: %d\n", fmt->ff_rsvdseccount);
+      fdbg("ERROR: Invalid number of reserved sectors: %d\n",
+           fmt->ff_rsvdseccount);
+
       ret = -EINVAL;
       goto errout;
     }
@@ -245,7 +263,7 @@ int mkfatfs(FAR const char *pathname, FAR struct fat_format_s *fmt)
   ret = open_blockdriver(pathname, 0, &var.fv_inode);
   if (ret < 0)
     {
-      fdbg("Failed to open %s\n", pathname);
+      fdbg("ERROR: Failed to open %s\n", pathname);
       goto errout;
     }
 
@@ -253,7 +271,9 @@ int mkfatfs(FAR const char *pathname, FAR struct fat_format_s *fmt)
 
   if (!var.fv_inode->u.i_bops->write || !var.fv_inode->u.i_bops->geometry)
     {
-      fdbg("%s does not support write or geometry methods\n", pathname);
+      fdbg("ERROR: %s does not support write or geometry methods\n",
+            pathname);
+
       ret = -EACCES;
       goto errout_with_driver;
     }
@@ -281,7 +301,7 @@ int mkfatfs(FAR const char *pathname, FAR struct fat_format_s *fmt)
   var.fv_sect = (uint8_t*)kmalloc(var.fv_sectorsize);
   if (!var.fv_sect)
     {
-      fdbg("Failed to allocate working buffers\n");
+      fdbg("ERROR: Failed to allocate working buffers\n");
       goto errout_with_driver;
     }
 

include/nuttx/fs/mkfatfs.h

@@ -114,7 +114,9 @@ extern "C" {
  * Name: mkfatfs
  *
  * Description:
- *   Make a FAT file system image on the specified block device
+ *   Make a FAT file system image on the specified block device.  This
+ *   function can automatically format a FAT12 or FAT16 file system.  By
+ *   tradition, FAT32 will only be selected is explicitly requested.
  *
  * Inputs:
  *   pathname - the full path to a registered block driver