Align dirents to sector boundary; fix scanning empty dirents (#15)

* Align dirents to sector boundary; fix scanning empty dirents

* Fix size computation

* Fix lints

xdvdfs-cli/src/cmd_info.rs

@@ -60,6 +60,18 @@ pub fn cmd_info(img_path: &String, entry: Option<&String>) -> Result<(), String>
                 .walk_path(&mut img, path)
                 .map_err(|e| e.to_string())?;
             print_dirent(&dirent);
+
+            if let Some(subdir) = dirent.node.dirent.dirent_table() {
+                println!();
+                let children = subdir
+                    .walk_dirent_tree(&mut img)
+                    .map_err(|e| e.to_string())?;
+                for node in children {
+                    println!("{}", node.get_name());
+                    print_dirent(&node);
+                    println!();
+                }
+            }
         }
         None => print_volume(&volume),
     }

xdvdfs-core/src/read.rs

@@ -24,11 +24,16 @@ pub fn read_volume<E>(
 fn read_dirent<E>(
     dev: &mut impl BlockDeviceRead<E>,
     offset: usize,
-) -> Result<DirectoryEntryNode, util::Error<E>> {
+) -> Result<Option<DirectoryEntryNode>, util::Error<E>> {
     let mut dirent_buf = [0; 0xe];
     dev.read(offset, &mut dirent_buf)
         .map_err(|e| util::Error::IOError(e))?;
 
+    // Empty directory entries are filled with 0xff
+    if dirent_buf == [0xff; 0xe] {
+        return Ok(None);
+    }
+
     let node = DirectoryEntryDiskNode::deserialize(&dirent_buf)?;
 
     let mut dirent = DirectoryEntryNode {
@@ -41,14 +46,14 @@ fn read_dirent<E>(
     dev.read(offset + 0xe, name_buf)
         .map_err(|e| util::Error::IOError(e))?;
 
-    Ok(dirent)
+    Ok(Some(dirent))
 }
 
 impl VolumeDescriptor {
     pub fn root_dirent<E>(
         &self,
         dev: &mut impl BlockDeviceRead<E>,
-    ) -> Result<DirectoryEntryNode, util::Error<E>> {
+    ) -> Result<Option<DirectoryEntryNode>, util::Error<E>> {
         if self.root_table.is_empty() {
             return Err(util::Error::DirectoryEmpty);
         }
@@ -67,6 +72,7 @@ impl DirectoryEntryTable {
 
         loop {
             let dirent = read_dirent(dev, offset)?;
+            let dirent = dirent.ok_or(util::Error::DoesNotExist)?;
             dprintln!(
                 "[find_dirent] Found {}: {:?}",
                 dirent.get_name(),
@@ -147,15 +153,26 @@ impl DirectoryEntryTable {
             let offset = self.offset(top)?;
             let dirent = read_dirent(dev, offset)?;
 
-            if dirent.node.left_entry_offset != 0 {
-                stack.push(4 * dirent.node.left_entry_offset as u32);
-            }
+            if let Some(dirent) = dirent {
+                dprintln!(
+                    "Found dirent {}: {:?} at offset {}",
+                    dirent.get_name(),
+                    dirent,
+                    top
+                );
+
+                let left_child = dirent.node.left_entry_offset;
+                if left_child != 0 && left_child != 0xffff {
+                    stack.push(4 * dirent.node.left_entry_offset as u32);
+                }
 
-            if dirent.node.right_entry_offset != 0 {
-                stack.push(4 * dirent.node.right_entry_offset as u32);
-            }
+                let right_child = dirent.node.right_entry_offset;
+                if right_child != 0 && right_child != 0xffff {
+                    stack.push(4 * dirent.node.right_entry_offset as u32);
+                }
 
-            dirents.push(dirent);
+                dirents.push(dirent);
+            }
         }
 
         Ok(dirents)
@@ -174,6 +191,7 @@ impl DirectoryEntryTable {
 
         let mut stack = vec![(String::from(""), *self)];
         while let Some((parent, tree)) = stack.pop() {
+            dprintln!("Descending through {}", parent);
             let children = tree.walk_dirent_tree(dev)?;
             for child in children.iter() {
                 if let Some(dirent_table) = child.node.dirent.dirent_table() {

xdvdfs-core/src/util.rs

@@ -96,4 +96,29 @@ mod test {
 
         assert_eq!(strings, ["abb", "a_b"]);
     }
+
+    #[test]
+    fn test_str_ignore_case_ordering() {
+        let mut strings = [
+            "NFL.png",
+            "NFLFever.jpg",
+            "NFLFever-noESRB.jpg",
+            "NFLFevertrial.xbe",
+            "NFL-noESRB.png",
+            "art",
+        ];
+        strings.sort_by(|a, b| cmp_ignore_case_utf8(a, b));
+
+        assert_eq!(
+            strings,
+            [
+                "art",
+                "NFL-noESRB.png",
+                "NFL.png",
+                "NFLFever-noESRB.jpg",
+                "NFLFever.jpg",
+                "NFLFevertrial.xbe",
+            ]
+        );
+    }
 }

xdvdfs-core/src/write/dirtab.rs

@@ -1,5 +1,5 @@
 use crate::layout::{
-    DirectoryEntryData, DirectoryEntryDiskData, DirectoryEntryDiskNode, DirentAttributes,
+    self, DirectoryEntryData, DirectoryEntryDiskData, DirectoryEntryDiskNode, DirentAttributes,
     DiskRegion,
 };
 use crate::util;
@@ -8,14 +8,14 @@ use crate::write::avl;
 use alloc::string::ToString;
 use alloc::{boxed::Box, string::String, vec::Vec};
 
-use super::sector::SectorAllocator;
+use super::sector::{required_sectors, SectorAllocator};
 
 /// Writer for directory entry tables
 #[derive(Default)]
 pub struct DirectoryEntryTableWriter {
     table: avl::AvlTree<DirectoryEntryData>,
 
-    size: usize,
+    size: Option<usize>,
 }
 
 pub struct FileListingEntry {
@@ -29,6 +29,16 @@ pub struct DirectoryEntryTableDiskRepr {
     pub file_listing: Vec<FileListingEntry>,
 }
 
+fn sector_align(offset: usize, incr: usize) -> usize {
+    let used_sectors = required_sectors(offset);
+    let needed_sectors = required_sectors(offset + incr);
+    if offset % layout::SECTOR_SIZE > 0 && needed_sectors > used_sectors {
+        layout::SECTOR_SIZE - offset % layout::SECTOR_SIZE
+    } else {
+        0
+    }
+}
+
 impl DirectoryEntryTableWriter {
     fn add_node<E>(
         &mut self,
@@ -51,7 +61,7 @@ impl DirectoryEntryTableWriter {
             name,
         };
 
-        self.size += dirent.len_on_disk();
+        self.size = None;
         self.table.insert(dirent);
 
         Ok(())
@@ -60,7 +70,7 @@ impl DirectoryEntryTableWriter {
     pub fn add_dir<E>(&mut self, name: &str, size: u32) -> Result<(), util::Error<E>> {
         let attributes = DirentAttributes(0).with_directory(true);
 
-        let size = size + (2048 - size % 2048);
+        let size = size + ((2048 - size % 2048) % 2048);
         self.add_node(name, size, attributes)
     }
 
@@ -69,9 +79,25 @@ impl DirectoryEntryTableWriter {
         self.add_node(name, size, attributes)
     }
 
+    pub fn compute_size(&mut self) {
+        self.size = Some(
+            self.table
+                .preorder_iter()
+                .map(|node| node.len_on_disk())
+                .fold(0, |acc, disk_len: usize| {
+                    acc + disk_len + sector_align(acc, disk_len)
+                }),
+        );
+    }
+
     /// Returns the size of the directory entry table, in bytes.
-    pub fn dirtab_size(&self) -> usize {
-        self.size
+    pub fn dirtab_size(&self) -> Option<usize> {
+        // FS bug: zero sized dirents are listed as size 2048
+        if self.table.backing_vec().is_empty() {
+            Some(2048)
+        } else {
+            self.size
+        }
     }
 
     /// Serializes directory entry table to a on-disk representation
@@ -105,9 +131,21 @@ impl DirectoryEntryTableWriter {
         }
 
         offsets.rotate_right(1);
+        let final_dirent_size = offsets[0];
         offsets[0] = 0;
         for i in 1..offsets.len() {
             offsets[i] += offsets[i - 1];
+
+            let next_size = if i + 1 == offsets.len() {
+                final_dirent_size
+            } else {
+                offsets[i + 1]
+            };
+            let adj: u16 = sector_align(offsets[i] as usize, next_size as usize)
+                .try_into()
+                .unwrap();
+            offsets[i] += adj;
+
             assert!(offsets[i] % 4 == 0);
         }
 
@@ -147,6 +185,12 @@ impl DirectoryEntryTableWriter {
                 name.as_bytes().len(),
                 dirent.dirent.filename_length as usize
             );
+
+            if dirent_bytes.len() < offsets[idx] as usize {
+                let offset = offsets[idx] as usize;
+                let diff = offset - dirent_bytes.len();
+                dirent_bytes.extend_from_slice(&alloc::vec![0xff; diff]);
+            }
             assert_eq!(dirent_bytes.len(), offsets[idx] as usize);
 
             dirent_bytes.extend_from_slice(&bytes);
@@ -158,6 +202,13 @@ impl DirectoryEntryTableWriter {
             }
         }
 
+        if let Some(size) = self.size {
+            assert_eq!(dirent_bytes.len(), size);
+        } else {
+            self.compute_size();
+            assert_eq!(Some(dirent_bytes.len()), self.size);
+        }
+
         Ok(DirectoryEntryTableDiskRepr {
             entry_table: dirent_bytes.into_boxed_slice(),
             file_listing,

xdvdfs-core/src/write/img.rs

@@ -66,7 +66,11 @@ pub fn create_xdvdfs_image<H: BlockDeviceWrite<E>, E>(
 
             match entry.file_type {
                 fs::FileType::Directory => {
-                    let dir_size = dirent_tables.get(&entry.path).unwrap().dirtab_size();
+                    let dir_size = dirent_tables
+                        .get(&entry.path)
+                        .unwrap()
+                        .dirtab_size()
+                        .unwrap();
                     let dir_size = dir_size.try_into().unwrap();
                     dirtab.add_dir(file_name, dir_size)?;
                 }
@@ -77,6 +81,7 @@ pub fn create_xdvdfs_image<H: BlockDeviceWrite<E>, E>(
             }
         }
 
+        dirtab.compute_size();
         dirent_tables.insert(path, dirtab);
     }
 
@@ -85,9 +90,9 @@ pub fn create_xdvdfs_image<H: BlockDeviceWrite<E>, E>(
     let mut sector_allocator = sector::SectorAllocator::default();
 
     let root_dirtab = dirent_tables.first_key_value().unwrap();
-    let root_sector = sector_allocator.allocate_contiguous(root_dirtab.1.dirtab_size());
-    let root_table =
-        layout::DirectoryEntryTable::new(root_dirtab.1.dirtab_size() as u32, root_sector as u32);
+    let root_ditab_size = root_dirtab.1.dirtab_size().unwrap();
+    let root_sector = sector_allocator.allocate_contiguous(root_ditab_size);
+    let root_table = layout::DirectoryEntryTable::new(root_ditab_size as u32, root_sector as u32);
     dir_sectors.insert(root_dirtab.0.to_path_buf(), root_sector);
 
     for (path, dirtab) in dirent_tables.into_iter() {
@@ -103,7 +108,7 @@ pub fn create_xdvdfs_image<H: BlockDeviceWrite<E>, E>(
 
         let dirtab_len = dirtab.entry_table.len();
         let padding_len = 2048 - dirtab_len % 2048;
-        if padding_len < 2048 {
+        if dirtab_len == 0 || padding_len < 2048 {
             let padding = vec![0xff; padding_len];
             BlockDeviceWrite::write(
                 image,
@@ -124,7 +129,7 @@ pub fn create_xdvdfs_image<H: BlockDeviceWrite<E>, E>(
                         as usize;
                 let padding_len = 2048 - file_len % 2048;
                 if padding_len < 2048 {
-                    let padding = vec![0xff; padding_len];
+                    let padding = vec![0; padding_len];
                     BlockDeviceWrite::write(
                         image,
                         entry.sector * layout::SECTOR_SIZE + file_len,

xdvdfs-core/src/write/sector.rs

@@ -11,11 +11,19 @@ impl Default for SectorAllocator {
     }
 }
 
+pub fn required_sectors(len: usize) -> usize {
+    if len != 0 {
+        len / SECTOR_SIZE + if len % SECTOR_SIZE > 0 { 1 } else { 0 }
+    } else {
+        1
+    }
+}
+
 impl SectorAllocator {
     /// Allocates a contiguous set of sectors, big enough to fit `bytes`.
     /// Returns the number of the first sector in the allocation
     pub fn allocate_contiguous(&mut self, bytes: usize) -> usize {
-        let sectors = bytes / SECTOR_SIZE + if bytes % SECTOR_SIZE > 0 { 1 } else { 0 };
+        let sectors = required_sectors(bytes);
         let allocation = self.next_free;
         self.next_free += sectors;
         allocation