Refactors VFile::is_seekable to use VFile::vnode() instead (#708)

src/kernel/src/fs/dev/mod.rs

@@ -102,7 +102,7 @@ fn alloc_vnode(
                 .unwrap()
                 .upgrade()
                 .ok_or(AllocVnodeError::DeviceGone)?;
-            let vn = Vnode::new(mnt, VnodeType::Character, tag, backend);
+            let vn = Vnode::new(mnt, VnodeType::CharacterDevice, tag, backend);
 
             *vn.item_mut() = Some(VnodeItem::Device(dev));
             vn

src/kernel/src/fs/file.rs

@@ -16,15 +16,17 @@ use thiserror::Error;
 /// An implementation of `file` structure.
 #[derive(Debug)]
 pub struct VFile {
-    ty: VFileType,     // f_type
-    flags: VFileFlags, // f_flag
+    ty: VFileType,             // f_type
+    flags: VFileFlags,         // f_flag
+    vnode: Option<Arc<Vnode>>, // f_vnode
 }
 
 impl VFile {
     pub(super) fn new(ty: VFileType) -> Self {
         Self {
             ty,
             flags: VFileFlags::empty(),
+            vnode: None,
         }
     }
 
@@ -36,8 +38,10 @@ impl VFile {
         &mut self.flags
     }
 
-    pub fn vnode(&self) -> Option<Arc<Vnode>> {
-        todo!()
+    /// Checking if this returns `Some` is equivalent to when FreeBSD and the PS4 check
+    /// fp->f_ops->fo_flags & DFLAG_PASSABLE != 0, therefore we use this instead.
+    pub fn vnode(&self) -> Option<&Arc<Vnode>> {
+        self.vnode.as_ref()
     }
 
     /// See `dofileread` on the PS4 for a reference.
@@ -140,10 +144,6 @@ impl VFile {
             VFileType::Blockpool(bp) => bp.truncate(self, length, td),
         }
     }
-
-    pub fn is_seekable(&self) -> bool {
-        matches!(self.ty, VFileType::Vnode(_) | VFileType::Device(_))
-    }
 }
 
 impl Seek for VFile {

src/kernel/src/fs/host/vnode.rs

@@ -41,7 +41,7 @@ impl crate::fs::VnodeBackend for VnodeBackend {
         let mode = match vn.ty() {
             VnodeType::Directory(_) => Mode::new(0o555).unwrap(),
             VnodeType::File | VnodeType::Link => todo!(),
-            VnodeType::Character => unreachable!(), // Character devices should only be in devfs.
+            VnodeType::CharacterDevice => unreachable!(), // Character devices should only be in devfs.
         };
 
         Ok(VnodeAttrs::new(Uid::ROOT, Gid::ROOT, mode, size, u32::MAX))
@@ -149,6 +149,5 @@ enum LookupError {
     OpenFailed(#[source] std::io::Error),
 
     #[error("cannot get vnode")]
-    #[errno(EIO)]
     GetVnodeFailed(#[source] GetVnodeError),
 }

src/kernel/src/fs/mod.rs

@@ -608,7 +608,7 @@ impl Fs {
         let fd: i32 = i.args[0].try_into().unwrap();
         let ptr: *mut u8 = i.args[1].into();
         let len: usize = i.args[2].try_into().unwrap();
-        let offset: u64 = i.args[3].try_into().unwrap();
+        let offset: i64 = i.args[3].try_into().unwrap();
 
         let iovec = unsafe { IoVec::try_from_raw_parts(ptr, len) }?;
 
@@ -624,7 +624,7 @@ impl Fs {
         let fd: i32 = i.args[0].try_into().unwrap();
         let ptr: *mut u8 = i.args[1].into();
         let len: usize = i.args[2].try_into().unwrap();
-        let offset: u64 = i.args[3].try_into().unwrap();
+        let offset: i64 = i.args[3].try_into().unwrap();
 
         let iovec = unsafe { IoVec::try_from_raw_parts(ptr, len) }?;
 
@@ -640,23 +640,23 @@ impl Fs {
         let fd: i32 = i.args[0].try_into().unwrap();
         let iovec: *mut IoVec = i.args[1].into();
         let count: u32 = i.args[2].try_into().unwrap();
-        let offset: u64 = i.args[3].try_into().unwrap();
+        let offset: i64 = i.args[3].try_into().unwrap();
 
         let uio = unsafe { UioMut::copyin(iovec, count) }?;
 
         self.preadv(fd, uio, offset, td)
     }
 
-    fn preadv(&self, fd: i32, uio: UioMut, off: u64, td: &VThread) -> Result<SysOut, SysErr> {
+    fn preadv(&self, fd: i32, uio: UioMut, offset: i64, td: &VThread) -> Result<SysOut, SysErr> {
         let file = td.proc().files().get_for_read(fd)?;
 
-        if !file.is_seekable() {
-            return Err(SysErr::Raw(ESPIPE));
-        }
+        let vnode = file.vnode().ok_or(SysErr::Raw(ESPIPE))?;
 
-        // TODO: check vnode type
+        if offset < 0 && !vnode.is_character() {
+            return Err(SysErr::Raw(EINVAL));
+        }
 
-        let read = file.do_read(uio, Offset::Provided(off), Some(&td))?;
+        let read = file.do_read(uio, Offset::Provided(offset), Some(&td))?;
 
         Ok(read.into())
     }
@@ -665,23 +665,23 @@ impl Fs {
         let fd: i32 = i.args[0].try_into().unwrap();
         let iovec: *const IoVec = i.args[1].into();
         let count: u32 = i.args[2].try_into().unwrap();
-        let offset: u64 = i.args[3].try_into().unwrap();
+        let offset: i64 = i.args[3].try_into().unwrap();
 
         let uio = unsafe { Uio::copyin(iovec, count) }?;
 
         self.pwritev(fd, uio, offset, td)
     }
 
-    fn pwritev(&self, fd: i32, uio: Uio, off: u64, td: &VThread) -> Result<SysOut, SysErr> {
+    fn pwritev(&self, fd: i32, uio: Uio, offset: i64, td: &VThread) -> Result<SysOut, SysErr> {
         let file = td.proc().files().get_for_write(fd)?;
 
-        if !file.is_seekable() {
-            return Err(SysErr::Raw(ESPIPE));
-        }
+        let vnode = file.vnode().ok_or(SysErr::Raw(ESPIPE))?;
 
-        // TODO: check vnode type
+        if offset < 0 && !vnode.is_character() {
+            return Err(SysErr::Raw(EINVAL));
+        }
 
-        let written = file.do_write(uio, Offset::Provided(off), Some(&td))?;
+        let written = file.do_write(uio, Offset::Provided(offset), Some(&td))?;
 
         Ok(written.into())
     }
@@ -743,11 +743,7 @@ impl Fs {
 
         let file = td.proc().files().get(fd)?;
 
-        if !file.is_seekable() {
-            return Err(SysErr::Raw(ESPIPE));
-        }
-
-        let _vnode = file.vnode().expect("File is not backed by a vnode");
+        let vnode = file.vnode().ok_or(SysErr::Raw(ESPIPE))?;
 
         // check vnode type
 
@@ -950,7 +946,7 @@ pub struct FsConfig {
 #[derive(Debug)]
 pub enum Offset {
     Current,
-    Provided(u64),
+    Provided(i64),
 }
 
 #[derive(Debug)]

src/kernel/src/fs/vnode.rs

@@ -60,7 +60,7 @@ impl Vnode {
     }
 
     pub fn is_character(&self) -> bool {
-        matches!(self.ty, VnodeType::Character)
+        matches!(self.ty, VnodeType::CharacterDevice)
     }
 
     pub fn item(&self) -> GutexReadGuard<Option<VnodeItem>> {
@@ -182,11 +182,11 @@ pub enum VnodeItem {
 }
 
 /// An implementation of `vtype`.
-#[derive(Debug, Clone)]
+#[derive(Debug, Clone, PartialEq, Eq)]
 pub enum VnodeType {
     File,            // VREG
     Directory(bool), // VDIR
-    Character,       // VCHR
+    CharacterDevice, // VCHR
     Link,            // VLNK
 }