feat: Make sector size and total sectors optional for format_volume

* moved bytes_per_sector and total_sectors from FormatVolumeOptions::new
to a dedicated methods (it is optional now)
* calculate total number of sectors using seek() to disk end if not provided
* added more doc comments
* added assertions about disk handle position being zero

src/boot_sector.rs

@@ -258,6 +258,9 @@ impl BiosParameterBlock {
                 "Invalid BPB (result of FAT32 determination from total number of clusters and sectors_per_fat_16 field differs)",
             ));
         }
+        if fat_type == FatType::Fat32 && total_clusters > 0x0FFF_FFFF {
+            return Err(Error::new(ErrorKind::Other, "Invalid BPB (too many clusters)"));
+        }
 
         let bits_per_fat_entry = fat_type.bits_per_fat_entry();
         let total_fat_entries = self.sectors_per_fat() * self.bytes_per_sector as u32 * 8 / bits_per_fat_entry as u32;
@@ -517,10 +520,9 @@ fn determine_sectors_per_fat(
     sectors_per_fat as u32
 }
 
-fn format_bpb(options: &FormatVolumeOptions) -> io::Result<(BiosParameterBlock, FatType)> {
+fn format_bpb(options: &FormatVolumeOptions, total_sectors: u32, bytes_per_sector: u16) -> io::Result<(BiosParameterBlock, FatType)> {
-    let bytes_per_sector = options.bytes_per_sector;
+    let total_bytes = u64::from(total_sectors) * u64::from(bytes_per_sector);
-    let total_sectors = options.total_sectors;
+    // FIXME: determine FAT type from cluster size because it can be modified by user
-    let total_bytes = total_sectors as u64 * bytes_per_sector as u64;
     let fat_type = options.fat_type.unwrap_or_else(|| determine_fat_type(total_bytes));
     let bytes_per_cluster = options
         .bytes_per_cluster
@@ -622,9 +624,9 @@ fn format_bpb(options: &FormatVolumeOptions) -> io::Result<(BiosParameterBlock,
     Ok((bpb, fat_type))
 }
 
-pub(crate) fn format_boot_sector(options: &FormatVolumeOptions) -> io::Result<(BootSector, FatType)> {
+pub(crate) fn format_boot_sector(options: &FormatVolumeOptions, total_sectors: u32, bytes_per_sector: u16) -> io::Result<(BootSector, FatType)> {
     let mut boot: BootSector = Default::default();
-    let (bpb, fat_type) = format_bpb(options)?;
+    let (bpb, fat_type) = format_bpb(options, total_sectors, bytes_per_sector)?;
     boot.bpb = bpb;
     boot.oem_name.copy_from_slice(b"MSWIN4.1");
     // Boot code copied from FAT32 boot sector initialized by mkfs.fat
@@ -717,7 +719,7 @@ mod tests {
         root_dir_entries: u32,
     ) {
         let total_sectors = total_bytes / u64::from(bytes_per_sector);
-        debug_assert!(total_sectors < u64::from(core::u32::MAX));
+        debug_assert!(total_sectors <= u64::from(core::u32::MAX), "{:x}", total_sectors);
         let total_sectors = total_sectors as u32;
 
         let sectors_per_cluster = (bytes_per_cluster / u32::from(bytes_per_sector)) as u8;
@@ -762,7 +764,7 @@ mod tests {
         let mut bytes_per_cluster = u32::from(bytes_per_sector);
         while bytes_per_cluster <= 64 * KB as u32 {
             let mut size = 1 * MB;
-            while size <= 2048 * GB {
+            while size < 2048 * GB {
                 test_determine_sectors_per_fat_single(
                     size,
                     bytes_per_sector,
@@ -774,6 +776,16 @@ mod tests {
                 );
                 size = size + size / 7;
             }
+            size = 2048 * GB - 1;
+            test_determine_sectors_per_fat_single(
+                size,
+                bytes_per_sector,
+                bytes_per_cluster,
+                fat_type,
+                reserved_sectors,
+                fats,
+                root_dir_entries,
+            );
             bytes_per_cluster *= 2;
         }
     }
@@ -796,4 +808,18 @@ mod tests {
         test_determine_sectors_per_fat_for_multiple_sizes(512, FatType::Fat32, 32, 1, 0);
         test_determine_sectors_per_fat_for_multiple_sizes(4096, FatType::Fat32, 32, 2, 0);
     }
+
+    #[test]
+    fn test_format_boot_sector_large_partition() {
+        let _ = env_logger::try_init();
+        let bytes_per_sector = 512;
+        let bytes_per_cluster = 4 * 4096;
+        let total_sectors = core::u32::MAX;
+        let (boot, _) = format_boot_sector(&FormatVolumeOptions::new()
+            .total_sectors(total_sectors)
+            .bytes_per_sector(bytes_per_sector)
+            .bytes_per_cluster(bytes_per_cluster), total_sectors, bytes_per_sector)
+            .expect("format_boot_sector");
+        boot.validate().expect("validate");
+    }
 }

src/fs.rs

@@ -789,8 +789,8 @@ fn write_zeros_until_end_of_sector<T: ReadWriteSeek>(mut disk: T, bytes_per_sect
 /// Options are specified as an argument for `format_volume` function.
 #[derive(Default, Debug, Clone)]
 pub struct FormatVolumeOptions {
-    pub(crate) bytes_per_sector: u16,
+    pub(crate) bytes_per_sector: Option<u16>,
-    pub(crate) total_sectors: u32,
+    pub(crate) total_sectors: Option<u32>,
     pub(crate) bytes_per_cluster: Option<u32>,
     pub(crate) fat_type: Option<FatType>,
     pub(crate) root_entries: Option<u16>,
@@ -803,21 +803,21 @@ pub struct FormatVolumeOptions {
 }
 
 impl FormatVolumeOptions {
-    /// Create options struct
+    /// Create options struct for `format_volume` function
     ///
-    /// `total_sectors` is size of partition in sectors.
+    /// Allows to overwrite many filesystem parameters.
-    /// `bytes_per_sector` is size of a logical sector (usually 512).
+    /// In normal use-case defaults should suffice.
-    /// Other options can be optionally specified by calling adequate methods.
+    pub fn new() -> Self {
-    /// If not specified suitable defaults will be used based on partition size.
-    pub fn new(total_sectors: u32, bytes_per_sector: u16) -> Self {
         FormatVolumeOptions {
-            total_sectors,
-            bytes_per_sector,
             ..Default::default()
         }
     }
 
     /// Set size of cluster in bytes (must be dividable by sector size)
+    ///
+    /// Cluster size must be a power of two and be greater or equal to sector size.
+    /// If option is not specified optimal cluster size is selected based on partition size and
+    /// optionally FAT type override (if specified using `fat_type` method).
     pub fn bytes_per_cluster(mut self, bytes_per_cluster: u32) -> Self {
         self.bytes_per_cluster = Some(bytes_per_cluster);
         self
@@ -825,14 +825,37 @@ impl FormatVolumeOptions {
 
     /// Set File Allocation Table type
     ///
-    /// Note: FAT type is defined by total number of clusters so changing this option
+    /// Option allows to override File Allocation Table (FAT) entry size.
-    /// can cause formatting to fail if volume size is incompatible.
+    /// It is unrecommended to set this option unless you know what you are doing.
+    /// Note: FAT type is determined from total number of clusters. Changing this option can cause formatting to fail
+    /// if the volume cannot be divided into proper number of clusters for selected FAT type.
     pub fn fat_type(mut self, fat_type: FatType) -> Self {
         self.fat_type = Some(fat_type);
         self
     }
 
+    /// Set sector size in bytes
+    ///
+    /// Sector size must be a power of two and be in range 512 - 4096.
+    /// Default is `512`.
+    pub fn bytes_per_sector(mut self, bytes_per_sector: u16) -> Self {
+        self.bytes_per_sector = Some(bytes_per_sector);
+        self
+    }
+
+    /// Set total number of sectors
+    ///
+    /// If option is not specified total number of sectors is calculated as storage device size divided by sector size.
+    pub fn total_sectors(mut self, total_sectors: u32) -> Self {
+        self.total_sectors = Some(total_sectors);
+        self
+    }
+
     /// Set maximal numer of entries in root directory for FAT12/FAT16 volumes
+    ///
+    /// Total root directory size should be dividable by sectors size so keep it a multiple of 16 (for default sector
+    /// size).
+    /// Default is `512`.
     pub fn root_entries(mut self, root_entries: u16) -> Self {
         self.root_entries = Some(root_entries);
         self
@@ -879,6 +902,8 @@ impl FormatVolumeOptions {
     }
 
     /// Set volume label
+    ///
+    /// Default is empty label.
     pub fn volume_label(mut self, volume_label: [u8; 11]) -> Self {
         self.volume_label = Some(volume_label);
         self
@@ -887,14 +912,31 @@ impl FormatVolumeOptions {
 
 /// Create FAT filesystem on a disk or partition (format a volume)
 ///
-/// Supplied `disk` parameter cannot be seeked. If there is a need to format a fragment of a disk
+/// Warning: this function overrides internal FAT filesystem structures and causes a loss of all data on provided
-/// image (e.g. partition) library user should wrap the file struct in a struct limiting
+/// partition. Please use it with caution.
-/// access to partition bytes only e.g. `fscommon::StreamSlice`.
+/// Only quick formatting is supported. To achieve a full format zero entire partition before calling this function.
+/// Supplied `disk` parameter cannot be seeked (internal pointer must be on position 0).
+/// To format a fragment of a disk image (e.g. partition) library user should wrap the file struct in a struct
+/// limiting access to partition bytes only e.g. `fscommon::StreamSlice`.
 pub fn format_volume<T: ReadWriteSeek>(mut disk: T, options: FormatVolumeOptions) -> io::Result<()> {
     trace!("format_volume");
+    debug_assert!(disk.seek(SeekFrom::Current(0))? == 0);
+
+    let bytes_per_sector = options.bytes_per_sector.unwrap_or(512);
+    let total_sectors = if options.total_sectors.is_none() {
+        let total_bytes: u64 = disk.seek(SeekFrom::End(0))?;
+        let total_sectors_64 = total_bytes / u64::from(bytes_per_sector);
         disk.seek(SeekFrom::Start(0))?;
+        if total_sectors_64 > u64::from(core::u32::MAX) {
+            return Err(Error::new(ErrorKind::Other, "Volume has too many sectors"));
+        }
+        total_sectors_64 as u32
+    } else {
+        options.total_sectors.unwrap() // SAFE: checked above
+    };
+
     // Create boot sector, validate and write to storage device
-    let (boot, fat_type) = format_boot_sector(&options)?;
+    let (boot, fat_type) = format_boot_sector(&options, total_sectors, bytes_per_sector)?;
     boot.validate()?;
     boot.serialize(&mut disk)?;
     // Make sure entire logical sector is updated (serialize method always writes 512 bytes)

tests/format.rs

@@ -74,7 +74,7 @@ fn test_format_fs(opts: fatfs::FormatVolumeOptions, total_bytes: u64) -> FileSys
 #[test]
 fn test_format_1mb() {
     let total_bytes = 1 * MB;
-    let opts = fatfs::FormatVolumeOptions::new((total_bytes / 512) as u32, 512);
+    let opts = fatfs::FormatVolumeOptions::new();
     let fs = test_format_fs(opts, total_bytes);
     assert_eq!(fs.fat_type(), fatfs::FatType::Fat12);
 }
@@ -82,7 +82,7 @@ fn test_format_1mb() {
 #[test]
 fn test_format_8mb() {
     let total_bytes = 8 * MB;
-    let opts = fatfs::FormatVolumeOptions::new((total_bytes / 512) as u32, 512);
+    let opts = fatfs::FormatVolumeOptions::new();
     let fs = test_format_fs(opts, total_bytes);
     assert_eq!(fs.fat_type(), fatfs::FatType::Fat16);
 }
@@ -90,7 +90,7 @@ fn test_format_8mb() {
 #[test]
 fn test_format_50mb() {
     let total_bytes = 50 * MB;
-    let opts = fatfs::FormatVolumeOptions::new((total_bytes / 512) as u32, 512);
+    let opts = fatfs::FormatVolumeOptions::new();
     let fs = test_format_fs(opts, total_bytes);
     assert_eq!(fs.fat_type(), fatfs::FatType::Fat16);
 }
@@ -98,20 +98,15 @@ fn test_format_50mb() {
 #[test]
 fn test_format_512mb_512sec() {
     let total_bytes = 2 * 1024 * MB;
-    let opts = fatfs::FormatVolumeOptions::new((total_bytes / 512) as u32, 512);
+    let opts = fatfs::FormatVolumeOptions::new();
     let fs = test_format_fs(opts, total_bytes);
     assert_eq!(fs.fat_type(), fatfs::FatType::Fat32);
 }
 
-fn create_format_options(total_bytes: u64, bytes_per_sector: Option<u16>) -> fatfs::FormatVolumeOptions {
-    let total_sectors = (total_bytes / bytes_per_sector.unwrap_or(512) as u64) as u32;
-    fatfs::FormatVolumeOptions::new(total_sectors, bytes_per_sector.unwrap_or(512))
-}
-
 #[test]
 fn test_format_512mb_4096sec() {
     let total_bytes = 1 * 1024 * MB;
-    let opts = create_format_options(total_bytes, Some(1024));
+    let opts = fatfs::FormatVolumeOptions::new().bytes_per_sector(4096);
     let fs = test_format_fs(opts, total_bytes);
     assert_eq!(fs.fat_type(), fatfs::FatType::Fat32);
 }