Added tests for mkdir_syscall (#255)

* Added tests for mkdir_syscall

* Added symlink test for mkdir_syscall

* Updated symlink test for mkdir_syscall

* Updated fs_mkdir call with comments and updated test

* Added information as comments

Author: namanlalitnyu

src/safeposix/syscalls/fs_calls.rs

@@ -181,72 +181,102 @@ impl Cage {
     }
 
     //------------------MKDIR SYSCALL------------------
-
+    // Description
+    // The mkdir_syscall() creates a new directory named by the path name pointed to by a path as the input parameter in the function.
+    // The mode of the new directory is initialized from the "mode" provided as the input parameter in the function.
+    // The newly created directory is empty with size 0 and is associated with a new inode of type "DIR".
+    // On successful completion, the timestamps for both the newly formed directory and its parent are updated along with their linkcounts.
+    
+    // Function Arguments
+    // The mkdir_syscall() receives two arguments:
+    // 1. Path - This represents the path at which the new directory will be created.
+    //           For example: "/parentdir/dir" represents the new directory name as "dir", which will be created at this path (/parentdir/dir).
+    // 2. Mode - This represents the permission of the newly created directory. 
+    //           The general mode used is "S_IRWXA": which represents the read, write, and search permissions on the new directory. 
+    
+    // Return Values
+    // Upon successful creation of the directory, 0 is returned.
+    // Otherwise, an error with a proper errorNumber and errorMessage is returned based on the different scenarios.
+    //
+    // Tests
+    // All the different scenarios for mkdir_syscall() are covered and tested in the "fs_tests.rs" file under "mkdir_syscall_tests" section.
+    //
     pub fn mkdir_syscall(&self, path: &str, mode: u32) -> i32 {
-        //Check that path is not empty
+
+        // Check that the given input path is not empty
         if path.len() == 0 {
             return syscall_error(Errno::ENOENT, "mkdir", "given path was null");
         }
-        let truepath = normpath(convpath(path), self);
 
-        //pass the metadata to this helper. If passed table is none, then create new instance
+        // Store the FileMetadata into a helper variable which is used for fetching the metadata of a given inode from the Inode Table. 
         let metadata = &FS_METADATA;
 
+        // Retrieve the absolute path from the root directory. The absolute path is then used to validate directory paths
+        // while navigating through subdirectories and establishing new directory at the given location.
+        let truepath = normpath(convpath(path), self);
+
+        // Walk through the absolute path which returns a tuple consisting of inode number of file (if it exists), and inode number of parent (if it exists)
         match metawalkandparent(truepath.as_path()) {
-            //If neither the file nor parent exists
+            // Case 1: When neither the file directory nor the parent directory exists
             (None, None) => syscall_error(
                 Errno::ENOENT,
                 "mkdir",
                 "a directory component in pathname does not exist or is a dangling symbolic link",
             ),
 
-            //If the file doesn't exist but the parent does
+            // Case 2: When the file doesn't exist but the parent directory exists
             (None, Some(pardirinode)) => {
                 let filename = truepath.file_name().unwrap().to_str().unwrap().to_string(); //for now we assume this is sane, but maybe this should be checked later
 
                 let effective_mode = S_IFDIR as u32 | mode;
-
-                //assert sane mode bits
+                // Check for the condition if the mode bits are correct and have the required permissions to create a directory
                 if mode & (S_IRWXA | S_FILETYPEFLAGS as u32) != mode {
                     return syscall_error(Errno::EPERM, "mkdir", "Mode bits were not sane");
                 }
 
+                // Fetch the next available inode number using the FileSystem MetaData table
+                // Create a new inode of type "Dir" representing a directory and set the required attributes
                 let newinodenum = FS_METADATA
                     .nextinode
                     .fetch_add(1, interface::RustAtomicOrdering::Relaxed); //fetch_add returns the previous value, which is the inode number we want
                 let time = interface::timestamp(); //We do a real timestamp now
-
                 let newinode = Inode::Dir(DirectoryInode {
-                    size: 0,
+                    size: 0, //initial size of a directory is 0 as it is empty
                     uid: DEFAULT_UID,
                     gid: DEFAULT_GID,
                     mode: effective_mode,
-                    linkcount: 3,
-                    refcount: 0, //2 because ., and .., as well as reference in parent directory
+                    linkcount: 3, //because of the directory name(.), itself, and reference to the parent directory(..)
+                    refcount: 0, //because no file descriptors are pointing to it currently
                     atime: time,
                     ctime: time,
                     mtime: time,
-                    filename_to_inode_dict: init_filename_to_inode_dict(newinodenum, pardirinode),
+                    filename_to_inode_dict: init_filename_to_inode_dict(newinodenum, pardirinode), //Establish a mapping between the newly created inode and the parent directory inode for easy retrieval and linking
                 });
 
-                if let Inode::Dir(ref mut parentdir) =
-                    *(metadata.inodetable.get_mut(&pardirinode).unwrap())
+                // Insert a reference to the file in the parent directory and update the inode attributes
+                // Fetch the inode of the parent directory and only proceed when its type is directory.
+                if let Inode::Dir(ref mut parentdir) = *(metadata.inodetable.get_mut(&pardirinode).unwrap())
                 {
                     parentdir
                         .filename_to_inode_dict
                         .insert(filename, newinodenum);
-                    parentdir.linkcount += 1;
+                    parentdir.linkcount += 1; // Since the parent is now associated to the new directory, its linkcount will increment by 1
+                    parentdir.ctime = time; // Here, update the ctime and mtime for the parent directory as well
+                    parentdir.mtime = time;
                 }
-                //insert a reference to the file in the parent directory
                 else {
                     unreachable!();
                 }
+                // Update the inode table by inserting the newly formed inode mapped with its inode number.
                 metadata.inodetable.insert(newinodenum, newinode);
                 log_metadata(&metadata, pardirinode);
                 log_metadata(&metadata, newinodenum);
-                0 //mkdir has succeeded
+
+                // Return 0 when mkdir has succeeded
+                0 
             }
 
+            // Case 3: When the file directory name already exists, then return the error.
             (Some(_), ..) => syscall_error(
                 Errno::EEXIST,
                 "mkdir",

src/tests/fs_tests.rs

@@ -50,6 +50,14 @@ pub mod fs_tests {
         ut_lind_fs_sem_trytimed();
         ut_lind_fs_sem_test();
         ut_lind_fs_tmp_file_test();
+
+        //mkdir_syscall_tests
+        ut_lind_fs_mkdir_empty_directory();
+        ut_lind_fs_mkdir_nonexisting_directory();
+        ut_lind_fs_mkdir_existing_directory();
+        ut_lind_fs_mkdir_invalid_modebits();
+        ut_lind_fs_mkdir_success();
+        ut_lind_fs_mkdir_using_symlink();
     }
 
     pub fn ut_lind_fs_simple() {
@@ -1237,6 +1245,98 @@ pub mod fs_tests {
         // Check if file is still there (it shouldn't be, assert no)
         assert_eq!(cage.access_syscall(file_path, F_OK), -2);
 
+        lindrustfinalize();
+    }   
+
+    pub fn ut_lind_fs_mkdir_empty_directory() {
+        lindrustinit(0);
+        let cage = interface::cagetable_getref(1);
+        let path = "";
+        // Check for error when directory is empty
+        assert_eq!(cage.mkdir_syscall(path, S_IRWXA), -(Errno::ENOENT as i32));
+        assert_eq!(cage.exit_syscall(EXIT_SUCCESS), EXIT_SUCCESS);
+        lindrustfinalize();
+    }
+
+    pub fn ut_lind_fs_mkdir_nonexisting_directory() {
+        lindrustinit(0);
+        let cage = interface::cagetable_getref(1);
+        let path = "/parentdir/dir";
+        // Check for error when both parent and child directories don't exist 
+        assert_eq!(cage.mkdir_syscall(path, S_IRWXA), -(Errno::ENOENT as i32));
+        assert_eq!(cage.exit_syscall(EXIT_SUCCESS), EXIT_SUCCESS);
+        lindrustfinalize();
+    }
+
+    pub fn ut_lind_fs_mkdir_existing_directory() {
+        lindrustinit(0);
+        let cage = interface::cagetable_getref(1);
+        let path = "/parentdir";
+        // Create a parent directory
+        cage.mkdir_syscall(path, S_IRWXA);
+        // Check for error when the same directory is created again
+        assert_eq!(cage.mkdir_syscall(path, S_IRWXA), -(Errno::EEXIST as i32));
+        assert_eq!(cage.exit_syscall(EXIT_SUCCESS), EXIT_SUCCESS);
+        lindrustfinalize();
+    }
+
+    pub fn ut_lind_fs_mkdir_invalid_modebits() {
+        lindrustinit(0);
+        let cage = interface::cagetable_getref(1);
+        let path = "/parentdir";
+        let invalid_mode = 0o77777; // Invalid mode bits
+        // Create a parent directory
+        cage.mkdir_syscall(path, S_IRWXA);
+        // Check for error when a directory is being created with invalid mode
+        assert_eq!(cage.mkdir_syscall("/parentdir/dir", invalid_mode), -(Errno::EPERM as i32));
+        assert_eq!(cage.exit_syscall(EXIT_SUCCESS), EXIT_SUCCESS);
+        lindrustfinalize();
+    }
+
+    pub fn ut_lind_fs_mkdir_success() {
+        lindrustinit(0);
+        let cage = interface::cagetable_getref(1);
+        let path = "/parentdir";
+        // Create a parent directory
+        cage.mkdir_syscall(path, S_IRWXA);
+
+        // Get the stat data for the parent directory and check for inode link count to be 3 initially
+        let mut statdata = StatData::default();
+        assert_eq!(cage.stat_syscall(path, &mut statdata), 0);
+        assert_eq!(statdata.st_nlink, 3);
+
+        // Create a child directory inside parent directory with valid mode bits
+        assert_eq!(cage.mkdir_syscall("/parentdir/dir", S_IRWXA), 0);
+        
+        // Get the stat data for the child directory and check for inode link count to be 3 initially
+        let mut statdata2 = StatData::default();
+        assert_eq!(cage.stat_syscall("/parentdir/dir", &mut statdata2), 0);
+        assert_eq!(statdata2.st_nlink, 3);
+
+        // Get the stat data for the parent directory and check for inode link count to be 4 now as a new child directory has been created.
+        let mut statdata3 = StatData::default();
+        assert_eq!(cage.stat_syscall(path, &mut statdata3), 0);
+        assert_eq!(statdata3.st_nlink, 4);
+
+        assert_eq!(cage.exit_syscall(EXIT_SUCCESS), EXIT_SUCCESS);
+        lindrustfinalize();
+    }
+
+    pub fn ut_lind_fs_mkdir_using_symlink() {
+        lindrustinit(0);
+        let cage = interface::cagetable_getref(1);
+
+        // Create a file which will be referred to as originalFile 
+        let fd = cage.open_syscall("/originalFile", O_CREAT | O_EXCL | O_WRONLY, S_IRWXA);
+        assert_eq!(cage.write_syscall(fd, str2cbuf("hi"), 2), 2);
+        
+        // Create a link between two files where the symlinkFile is originally not present
+        // But while linking, symlinkFile will get created
+        assert_eq!(cage.link_syscall("/originalFile", "/symlinkFile"), 0);
+
+        // Check for error while creating the symlinkFile again as it would already be created while linking the two files above.
+        assert_eq!(cage.mkdir_syscall("/symlinkFile", S_IRWXA), -(Errno::EEXIST as i32));
+        assert_eq!(cage.exit_syscall(EXIT_SUCCESS), EXIT_SUCCESS);
         lindrustfinalize();
     }
 }