Fcntl_syscall updates

src/safeposix/syscalls/fs_calls.rs

@@ -1900,85 +1900,102 @@ impl Cage {
     //------------------------------------FCNTL SYSCALL------------------------------------
 
     pub fn fcntl_syscall(&self, fd: i32, cmd: i32, arg: i32) -> i32 {
-        let checkedfd = self.get_filedescriptor(fd).unwrap();
-        let mut unlocked_fd = checkedfd.write();
-        if let Some(filedesc_enum) = &mut *unlocked_fd {
-            let flags = match filedesc_enum {
-                Epoll(obj) => &mut obj.flags,
-                Pipe(obj) => &mut obj.flags,
-                Stream(obj) => &mut obj.flags,
-                File(obj) => &mut obj.flags,
-                Socket(ref mut sockfdobj) => {
-                    if cmd == F_SETFL && arg >= 0 {
-                        let sock_tmp = sockfdobj.handle.clone();
-                        let mut sockhandle = sock_tmp.write();
-
-                        if let Some(ins) = &mut sockhandle.innersocket {
-                            let fcntlret;
-                            if arg & O_NONBLOCK == O_NONBLOCK {
-                                //set for non-blocking I/O
-                                fcntlret = ins.set_nonblocking();
-                            } else {
-                                //clear non-blocking I/O
-                                fcntlret = ins.set_blocking();
-                            }
-                            if fcntlret < 0 {
-                                match Errno::from_discriminant(interface::get_errno()) {
-                                    Ok(i) => {
-                                        return syscall_error(
-                                            i,
-                                            "fcntl",
-                                            "The libc call to fcntl failed!",
-                                        );
-                                    }
-                                    Err(()) => panic!("Unknown errno value from fcntl returned!"),
-                                };
+        //if the provided file descriptor is out of bounds, get_filedescriptor returns Err(),
+        //and using unwrap on Err() causes a thread to panick
+        //instead, I propose using the 'if let' construct to be able to report an error to 
+        //the user instead of simply panicking
+        //otherwise, file descriptor table entry is stored in 'checkedfd'
+        if let Ok(checkedfd) = self.get_filedescriptor(fd) {        
+            let mut unlocked_fd = checkedfd.write();
+            //performing the specified command if the file descriptor entry is not empty
+            if let Some(filedesc_enum) = &mut *unlocked_fd {
+                //'flags' consists of bitwise-or'd access mode, file creation, and file status flags
+                //to retrieve a particular flag, it can bitwise-and'd with 'flags'
+                let flags = match filedesc_enum {
+                    Epoll(obj) => &mut obj.flags,
+                    Pipe(obj) => &mut obj.flags,
+                    Stream(obj) => &mut obj.flags,
+                    File(obj) => &mut obj.flags,
+                    //not clear why running F_SETFL on Socket type requires special treatment
+                    Socket(ref mut sockfdobj) => {
+                        if cmd == F_SETFL && arg >= 0 {
+                            let sock_tmp = sockfdobj.handle.clone();
+                            let mut sockhandle = sock_tmp.write();
+
+                            if let Some(ins) = &mut sockhandle.innersocket {
+                                let fcntlret;
+                                if arg & O_NONBLOCK == O_NONBLOCK {
+                                    //set for non-blocking I/O
+                                    fcntlret = ins.set_nonblocking();
+                                } else {
+                                    //clear non-blocking I/O
+                                    fcntlret = ins.set_blocking();
+                                }
+                                if fcntlret < 0 {
+                                    match Errno::from_discriminant(interface::get_errno()) {
+                                        Ok(i) => {
+                                            return syscall_error(
+                                                i,
+                                                "fcntl",
+                                                "The libc call to fcntl failed!",
+                                            );
+                                        }
+                                        Err(()) => panic!("Unknown errno value from fcntl returned!"),
+                                    };
+                                }
                             }
                         }
-                    }
 
-                    &mut sockfdobj.flags
-                }
-            };
+                        &mut sockfdobj.flags
+                    }
+                };
 
-            //matching the tuple
-            match (cmd, arg) {
-                //because the arg parameter is not used in certain commands, it can be anything (..)
-                (F_GETFD, ..) => *flags & O_CLOEXEC,
-                // set the flags but make sure that the flags are valid
-                (F_SETFD, arg) if arg >= 0 => {
-                    if arg & O_CLOEXEC != 0 {
-                        *flags |= O_CLOEXEC;
-                    } else {
-                        *flags &= !O_CLOEXEC;
+                //matching the tuple
+                match (cmd, arg) {
+                    //because the arg parameter is not used in certain commands, it can be anything (..)
+                    //currently, O_CLOEXEC is the only defined file descriptor flag, thus only this flag is
+                    //masked when using F_GETFD or F_SETFD
+                    (F_GETFD, ..) => *flags & O_CLOEXEC,
+                    // set the flags but make sure that the flags are valid
+                    (F_SETFD, arg) if arg >= 0 => {
+                        if arg & O_CLOEXEC != 0 {
+                            *flags |= O_CLOEXEC;
+                        } else {
+                            *flags &= !O_CLOEXEC;
+                        }
+                        0
                     }
-                    0
-                }
-                (F_GETFL, ..) => {
-                    //for get, we just need to return the flags
-                    *flags & !O_CLOEXEC
-                }
-                (F_SETFL, arg) if arg >= 0 => {
-                    *flags |= arg;
-                    0
-                }
-                (F_DUPFD, arg) if arg >= 0 => self._dup2_helper(&filedesc_enum, arg, false),
-                //TO DO: implement. this one is saying get the signals
-                (F_GETOWN, ..) => {
-                    0 //TO DO: traditional SIGIO behavior
-                }
-                (F_SETOWN, arg) if arg >= 0 => {
-                    0 //this would return the PID if positive and the process group if negative,
-                      //either way do nothing and return success
+                    //F_GETFL should return file access mode and the file status flags, thus excluding
+                    //the file creation flags. It is not clear why O_CLOEXEC is the only masked-out flag,
+                    //when there are other file creation flags, like O_CREAT, O_DIRECTORY, etc. 
+                    (F_GETFL, ..) => {
+                        //for get, we just need to return the flags
+                        *flags & !O_CLOEXEC
+                    }
+                    (F_SETFL, arg) if arg >= 0 => {
+                        *flags |= arg;
+                        0
+                    }
+                    (F_DUPFD, arg) if arg >= 0 => self._dup2_helper(&filedesc_enum, arg, false),
+                    //TO DO: implement. this one is saying get the signals
+                    (F_GETOWN, ..) => {
+                        0 //TO DO: traditional SIGIO behavior
+                    }
+                    (F_SETOWN, arg) if arg >= 0 => {
+                        0 //this would return the PID if positive and the process group if negative,
+                        //either way do nothing and return success
+                    }
+                    _ => syscall_error(
+                        Errno::EINVAL,
+                        "fcntl",
+                        "Arguments provided do not match implemented parameters",
+                    ),
                 }
-                _ => syscall_error(
-                    Errno::EINVAL,
-                    "fcntl",
-                    "Arguments provided do not match implemented parameters",
-                ),
+            } else {
+                syscall_error(Errno::EBADF, "fcntl", "Invalid file descriptor")
             }
         } else {
-            syscall_error(Errno::EBADF, "fcntl", "Invalid file descriptor")
+            syscall_error(Errno::EBADF, "fcntl", "File descriptor is out of range")
         }
     }
 

src/tests/fs_tests.rs

@@ -20,7 +20,10 @@ pub mod fs_tests {
         ut_lind_fs_dir_multiple();
         ut_lind_fs_dup();
         ut_lind_fs_dup2();
-        ut_lind_fs_fcntl();
+        ut_lind_fs_fcntl_valid_args();
+        ut_lind_fs_fcntl_invalid_args();
+        ut_lind_fs_fcntl_invalid_fd();
+        ut_lind_fs_fcntl_dup();
         ut_lind_fs_ioctl();
         ut_lind_fs_fdflags();
         ut_lind_fs_file_link_unlink();
@@ -437,35 +440,104 @@ pub mod fs_tests {
         lindrustfinalize();
     }
 
-    pub fn ut_lind_fs_fcntl() {
+    pub fn ut_lind_fs_fcntl_valid_args() {
         lindrustinit(0);
         let cage = interface::cagetable_getref(1);
 
         let sockfd = cage.socket_syscall(AF_INET, SOCK_STREAM, 0);
-        let filefd = cage.open_syscall("/fcntl_file", O_CREAT | O_EXCL, S_IRWXA);
+        let filefd = cage.open_syscall("/fcntl_file_1", O_CREAT | O_EXCL, S_IRWXA);
 
-        //set the setfd flag
+        //changing O_CLOEXEC file descriptor flag and checking if it was correctly set
         assert_eq!(cage.fcntl_syscall(sockfd, F_SETFD, O_CLOEXEC), 0);
-
-        //checking to see if the wrong flag was set or not
         assert_eq!(cage.fcntl_syscall(sockfd, F_GETFD, 0), O_CLOEXEC);
 
-        //let's get some more flags on the filefd
-        assert_eq!(
-            cage.fcntl_syscall(filefd, F_SETFL, O_RDONLY | O_NONBLOCK),
-            0
-        );
-
-        //checking if the flags are updated...
+        //changing the file access mode to read-only, enabling the 
+        //O_NONBLOCK file status flag, and checking if they were correctly set
+        assert_eq!(cage.fcntl_syscall(filefd, F_SETFL, O_RDONLY | O_NONBLOCK), 0);
         assert_eq!(cage.fcntl_syscall(filefd, F_GETFL, 0), 2048);
 
+        //when provided with 'F_GETFD' or 'F_GETFL' command, 'arg' should be ignored, thus even
+        //negative arg values should produce nomal behavior
+        //However, testing results in two errors
+        assert_eq!(cage.fcntl_syscall(sockfd, F_GETFD, -132), O_CLOEXEC);
+        assert_eq!(cage.fcntl_syscall(filefd, F_GETFL, -1998), 2048);
+
         assert_eq!(cage.close_syscall(filefd), 0);
         assert_eq!(cage.close_syscall(sockfd), 0);
 
         assert_eq!(cage.exit_syscall(EXIT_SUCCESS), EXIT_SUCCESS);
         lindrustfinalize();
     }
 
+    pub fn ut_lind_fs_fcntl_invalid_args(){
+        lindrustinit(0);
+        let cage = interface::cagetable_getref(1);
+        let filefd = cage.open_syscall("/fcntl_file_2", O_CREAT | O_EXCL, S_IRWXA);
+        //when presented with a nonexistent command, 'Invalid Argument' error should be thrown
+        //29 is an arbitrary number that does not correspond to any of the defined 'fcntl' commands
+        assert_eq!(cage.fcntl_syscall(filefd, 29, 0), -(Errno::EINVAL as i32));
+        //when a negative arg is provided with F_SETFD, F_SETFL, or F_DUPFD,
+        //Invalid Argument' error should be thrown as well
+        assert_eq!(cage.fcntl_syscall(filefd, F_SETFD, -5), -(Errno::EINVAL as i32));
+        assert_eq!(cage.fcntl_syscall(filefd, F_SETFL, -5), -(Errno::EINVAL as i32));
+        assert_eq!(cage.fcntl_syscall(filefd, F_DUPFD, -5), -(Errno::EINVAL as i32));
+
+        assert_eq!(cage.close_syscall(filefd), 0);
+
+        assert_eq!(cage.exit_syscall(EXIT_SUCCESS), EXIT_SUCCESS);
+        lindrustfinalize();
+    }
+
+    pub fn ut_lind_fs_fcntl_invalid_fd(){
+        lindrustinit(0);
+        let cage = interface::cagetable_getref(1);
+        //valid file descriptors range from 0 to 1024 (excluded)
+        //passing an invalid file descriptor outside of that range 
+        //should produce a 'Bad file number' error
+        assert_eq!(cage.fcntl_syscall(-10, F_GETFD, 0), -(Errno::EBADF as i32));
+        assert_eq!(cage.fcntl_syscall(2048, F_GETFD, 0), -(Errno::EBADF as i32));
+
+        //calling 'fcntl' on an unused file descriptor should throw 'Bad file number' error
+        let filefd = cage.open_syscall("/fcntl_file_3", O_CREAT | O_EXCL, S_IRWXA);
+        //since no other file is created inside the current thread right after 'close' is called
+        //on 'filefd', it should become unused
+        cage.close_syscall(filefd);
+        assert_eq!(cage.fcntl_syscall(filefd, F_SETFD, O_CLOEXEC), -(Errno::EBADF as i32));
+
+        assert_eq!(cage.exit_syscall(EXIT_SUCCESS), EXIT_SUCCESS);
+        lindrustfinalize();
+    }
+
+    pub fn ut_lind_fs_fcntl_dup(){
+        lindrustinit(0);
+        let cage = interface::cagetable_getref(1);
+
+        let filefd1 = cage.open_syscall("/fcntl_file_4", O_CREAT | O_EXCL | O_RDWR, S_IRWXA);
+        //on success, returning the new file descriptor greater than or equal to 100
+        //and different from the original file descriptor
+        let filefd2 = cage.fcntl_syscall(filefd1, F_DUPFD, 100);
+        assert!(filefd2 >= 100 && filefd2 != filefd1);
+
+        //to check if both file descriptors refer to the same fie, we can write into a file
+        //using one file descriptor, read from the file using another file descriptor,
+        //and make sure that the contents are the same
+        let mut temp_buffer = sizecbuf(9);
+        assert_eq!(cage.write_syscall(filefd1, str2cbuf("Test text"), 9), 9);
+        assert_eq!(cage.read_syscall(filefd2, temp_buffer.as_mut_ptr(), 9), 9);
+        assert_eq!(cbuf2str(&temp_buffer), "Test text");
+
+        //file status flags are shared by duplicated file descriptors resulting from
+        //a single opening of the file
+        assert_eq!(cage.fcntl_syscall(filefd1, F_GETFL, 0), cage.fcntl_syscall(filefd2, F_GETFL, 0));
+
+        assert_eq!(cage.close_syscall(filefd1), 0);
+        assert_eq!(cage.close_syscall(filefd2), 0);
+
+        assert_eq!(cage.exit_syscall(EXIT_SUCCESS), EXIT_SUCCESS);
+        lindrustfinalize();
+    }
+
+
     pub fn ut_lind_fs_ioctl() {
         lindrustinit(0);
         let cage = interface::cagetable_getref(1);