make sure that entry names are unique when adding them to a directory

When adding entries to a directory, we now make sure that their name is unique within that directory.

Author: tsatke

ext2/src/dir.rs

@@ -49,17 +49,28 @@ where
         Ok(entries)
     }
 
-    pub fn lookup_dir_entry<P>(
+    pub fn find_entry<P>(
         &self,
         dir: &Directory,
         p: P,
-    ) -> Result<Option<(InodeAddress, Inode)>, Error>
+    ) -> Result<Option<DirEntry>, Error>
     where
         P: FnMut(&DirEntry) -> bool,
     {
-        self.list_dir(dir)?
+        Ok(self.list_dir(dir)?
             .into_iter()
-            .find(p)
+            .find(p))
+    }
+
+    pub fn find_and_resolve_entry<P>(
+        &self,
+        dir: &Directory,
+        p: P,
+    ) -> Result<Option<(InodeAddress, Inode)>, Error>
+    where
+        P: FnMut(&DirEntry) -> bool,
+    {
+        self.find_entry(dir, p)?
             .map(|e| self.resolve_dir_entry(e))
             .transpose()
     }
@@ -75,6 +86,11 @@ where
         inode_address: InodeAddress,
         typ: DirType,
     ) -> Result<(), Error> {
+        // TODO: make this more efficient once we have indexed lookups implemented
+        if self.find_entry(dir, |e| e.name() == Some(name))?.is_some() {
+            return Err(Error::EntryExists);
+        }
+
         let block_size = self.superblock.block_size() as usize;
         let dir_entries_have_type = self
             .superblock

ext2/src/error.rs

@@ -14,6 +14,7 @@ pub enum Error {
     InvalidInodeAddress(u32),
     NoSpace,
     NotSupported,
+    EntryExists,
 }
 
 impl Display for Error {

ext2/src/lib.rs

@@ -28,6 +28,7 @@ mod write;
 
 const ROOT_DIR_INODE_ADDRESS: InodeAddress = InodeAddress::new(2).unwrap();
 
+/// An ext2 filesystem over a block device.
 pub struct Ext2Fs<T> {
     block_device: T,
     superblock: Superblock,

ext2/tests/common.rs

@@ -5,6 +5,17 @@ use std::path::{Path, PathBuf};
 use rand::distributions::Alphanumeric;
 use rand::Rng;
 
+#[macro_export]
+macro_rules! cow_fs {
+    ($path:literal, $device_sector_size:expr) => {
+        {
+            let image_data = common::load_copy_of_image($path);
+            let device = MemoryBlockDevice::try_new($device_sector_size, image_data).unwrap();
+            Ext2Fs::try_new(device).unwrap()
+        }
+    };
+}
+
 #[macro_export]
 macro_rules! generate_tests {
     ($test_fn:ident : $($size:literal - $name:ident),*,) => {

ext2/tests/read.rs

@@ -109,7 +109,7 @@ fn do_test_read_file(sector_size: usize) {
     let root = fs.read_root_inode().unwrap().try_into().unwrap();
 
     let hello_txt: RegularFile = fs
-        .lookup_dir_entry(&root, |e| e.name().is_some_and(|n| n == "hello.txt"))
+        .find_and_resolve_entry(&root, |e| e.name().is_some_and(|n| n == "hello.txt"))
         .expect("lookup_dir_entry failed")
         .expect("hello.txt not found")
         .try_into()

ext2/tests/write.rs

@@ -1,4 +1,4 @@
-use ext2::Ext2Fs;
+use ext2::{Error, Ext2Fs};
 use filesystem::MemoryBlockDevice;
 
 mod common;
@@ -13,9 +13,7 @@ generate_tests!(
 );
 
 fn test_create_and_write_file(sector_size: usize) {
-    let image_data = common::load_copy_of_image("tests/filesystems/empty.img");
-    let device = MemoryBlockDevice::try_new(sector_size, image_data).unwrap();
-    let mut fs = Ext2Fs::try_new(device).unwrap();
+    let mut fs = cow_fs!("tests/filesystems/empty.img", sector_size);
 
     let file_name = "my_file.txt";
     let mut root = fs.read_root_inode().unwrap();
@@ -28,4 +26,48 @@ fn test_create_and_write_file(sector_size: usize) {
     for i in 0..((1024 * 12) / data.len()) {
         assert_eq!(fs.write_to_file(&mut file, i * data.len(), data).unwrap(), data.len());
     }
+}
+
+generate_tests!(
+    test_create_files:
+    512 - test_create_files_standard,
+    1 - test_create_files_tiny,
+    32 - test_create_files_small,
+    32768 - test_create_files_large,
+    1048576 - test_create_files_huge,
+);
+
+fn test_create_files(sector_size: usize) {
+    let mut fs = cow_fs!("tests/filesystems/empty.img", sector_size);
+
+    let mut root = fs.read_root_inode().unwrap();
+    for i in 0..25 {
+        let file_name = format!("file_{}.txt", i);
+        let file = fs.create_regular_file(&mut root, &file_name).unwrap();
+        assert!(fs.list_dir(&root).unwrap().iter().find(|e| e.name() == Some(&file_name)).is_some());
+        assert_eq!(file.len(), 0);
+    }
+}
+
+generate_tests!(
+    test_create_file_collision:
+    512 - test_create_file_collision_standard,
+    1 - test_create_file_collision_tiny,
+    32 - test_create_file_collision_small,
+    32768 - test_create_file_collision_large,
+    1048576 - test_create_file_collision_huge,
+);
+
+fn test_create_file_collision(sector_size: usize) {
+    let mut fs = cow_fs!("tests/filesystems/empty.img", sector_size);
+
+    let mut root = fs.read_root_inode().unwrap();
+    let file_name = "file.txt";
+    let file = fs.create_regular_file(&mut root, file_name).unwrap();
+    assert!(fs.list_dir(&root).unwrap().iter().find(|e| e.name() == Some(file_name)).is_some());
+    assert_eq!(file.len(), 0);
+
+    let mut root = fs.read_root_inode().unwrap();
+    let result = fs.create_regular_file(&mut root, file_name);
+    assert_eq!(result.unwrap_err(), Error::EntryExists);
 }