From 8c9521e893993d5177dd2307b28eeccad4f30ab6 Mon Sep 17 00:00:00 2001
From: TheOddGarlic <umutinanerdogan@pm.me>
Date: Thu, 4 Aug 2022 13:19:05 +0300
Subject: [PATCH] VFS: simplify everything

---
 ableos/src/filesystem/ext2.rs | 17 +---------
 ableos/src/filesystem/mod.rs  | 63 +++--------------------------------
 ableos/src/kernel_state.rs    | 34 +++++++++++++++----
 3 files changed, 32 insertions(+), 82 deletions(-)

diff --git a/ableos/src/filesystem/ext2.rs b/ableos/src/filesystem/ext2.rs
index bf590c4a..adcc0ebc 100644
--- a/ableos/src/filesystem/ext2.rs
+++ b/ableos/src/filesystem/ext2.rs
@@ -4,13 +4,12 @@
  * SPDX-License-Identifier: MPL-2.0
  */
 
-use alloc::sync::Arc;
 use ext2::fs::{sync::Synced, Ext2};
 use ext2::sector::SectorSize;
 use ext2::volume::Volume;
 
 use super::errors::FsError;
-use super::{DirectoryEntry, FileDescriptor, FsNode, FsResult as Result, StorageDevice};
+use super::{DirectoryEntry, FsNode, FsResult as Result, StorageDevice};
 
 pub struct Ext2StorageDevice<S, V>
 where
@@ -37,20 +36,6 @@ where
     S: SectorSize + Send,
     V: Volume<u8, S> + Send,
 {
-    fn open(&self, node: Arc<FsNode> /* TODO: flags */) -> Result<FileDescriptor> {
-        let inode = self
-            .fs
-            .inode_nth(node.inode as usize)
-            .ok_or_else(|| FsError::InodeNotFound)?;
-
-        Ok(FileDescriptor::new(Arc::downgrade(&node), node.flags, inode.size(), node.inode))
-    }
-
-    fn release(&self, _node: FsNode) -> Result<()> {
-        // TODO: flush to disk and whatnot
-        Ok(())
-    }
-
     fn read(&self, node: &FsNode, offset: usize, size: usize, buffer: &mut Vec<u8>) -> Result<()> {
         let inode = self
             .fs
diff --git a/ableos/src/filesystem/mod.rs b/ableos/src/filesystem/mod.rs
index 96be6596..8793191c 100644
--- a/ableos/src/filesystem/mod.rs
+++ b/ableos/src/filesystem/mod.rs
@@ -24,12 +24,11 @@ pub trait StorageDevice
 where
     Self: Send,
 {
-    fn open(&self, node: Arc<FsNode> /* TODO: flags */) -> Result<FileDescriptor>;
-    fn release(&self, node: FsNode) -> Result<()>;
     fn read(&self, node: &FsNode, offset: usize, size: usize, buffer: &mut Vec<u8>) -> Result<()>;
     fn write(&self, node: &FsNode, offset: usize, buffer: &[u8]) -> Result<()>;
     fn read_dir(&self, node: &FsNode, index: usize) -> Result<DirectoryEntry>;
     fn find_dir(&self, node: &FsNode, name: &str) -> Result<FsNode>;
+    // TODO: flush to disk
 }
 
 /// A VFS node, that can either be a file or a directory.
@@ -69,31 +68,11 @@ impl FsNode {
     // TODO: make this take flags
     pub fn open(self: Arc<Self>) -> Result<Handle> {
         let mut state = KERNEL_STATE.lock();
-        let device = state
-            .get_storage_device(self.device_handle)
-            .ok_or_else(|| FsError::InvalidDevice)?;
-        let fd = device.open(self)?;
-        let handle = state.open_file_descriptor(fd);
+        let handle = state.open_file_descriptor(self);
 
         Ok(handle)
     }
 
-    /// This method is for closing the VFS node, which is usually done when no
-    /// open file descriptors for this file are left. File descriptors have
-    /// weak references to the VFS node, meaning it is okay to close the VFS
-    /// node before all file descriptors are closed.
-    pub fn release(self) -> Result<()> {
-        let state = KERNEL_STATE.lock();
-        let device = state
-            .get_storage_device(self.device_handle)
-            .ok_or_else(|| FsError::InvalidDevice)?;
-
-        device.release(self)?;
-        // self is moved into device.release, and at the end of it, self should
-        // be dropped.
-        Ok(())
-    }
-
     /// This method reads from the VFS node without opening a new file
     /// descriptor. This is intended to be used internally, if you're trying to
     /// read a file then you probably want to read from a file descriptor.
@@ -153,7 +132,8 @@ impl FsNode {
 
 impl Drop for FsNode {
     fn drop(&mut self) {
-        trace!("dropping: {self:#?}")
+        trace!("dropping VFS node: {self:#?}");
+        // TODO: flush to disk here
     }
 }
 
@@ -172,41 +152,6 @@ bitflags! {
     }
 }
 
-/// A file descriptor.
-#[derive(Debug)]
-pub struct FileDescriptor {
-    vfs_node: Weak<FsNode>, // ptr to the associated VFS node
-    flags: FsFlags,
-    length: usize, // in bytes
-    inode: usize,  // implementation specific identifier for the node
-                   // TODO: permissions mask
-                   // TODO: owning user/group
-                   // TODO: read file position?
-}
-
-impl FileDescriptor {
-    fn new(vfs_node: Weak<FsNode>, flags: FsFlags, length: usize, inode: usize) -> Self {
-        Self {
-            vfs_node,
-            flags,
-            length,
-            inode,
-        }
-    }
-
-    /// This method is for closing the file descriptor.
-    pub fn close(fd_handle: Handle) {
-        let mut state = KERNEL_STATE.lock();
-        state.close_file_descriptor(fd_handle);
-    }
-}
-
-impl Drop for FileDescriptor {
-    fn drop(&mut self) {
-        trace!("dropping: {self:#?}")
-    }
-}
-
 pub struct DirectoryEntry {
     name: String,
     inode: usize,
diff --git a/ableos/src/kernel_state.rs b/ableos/src/kernel_state.rs
index 2ca4bcdc..4cb69363 100644
--- a/ableos/src/kernel_state.rs
+++ b/ableos/src/kernel_state.rs
@@ -1,8 +1,9 @@
+use alloc::sync::Arc;
 use hashbrown::HashMap;
 use spin::Lazy;
 
 use crate::{
-    filesystem::{FileDescriptor, StorageDevice},
+    filesystem::{StorageDevice, FsNode},
     handle::{Handle, HandleResource},
 };
 
@@ -12,7 +13,8 @@ pub static KERNEL_STATE: Lazy<spin::Mutex<KernelInternalState>> =
 pub struct KernelInternalState {
     pub hostname: String,
     storage_devices: HashMap<Handle, Box<dyn StorageDevice>>,
-    fd_table: HashMap<Handle, FileDescriptor>,
+    // FIXME: should this be per-process?
+    file_table: HashMap<Handle, FileTableEntry>,
     should_shutdown: bool,
 }
 
@@ -21,7 +23,7 @@ impl KernelInternalState {
         Self {
             should_shutdown: false,
             storage_devices: HashMap::new(),
-            fd_table: HashMap::new(),
+            file_table: HashMap::new(),
             hostname: "".to_string(),
         }
     }
@@ -40,14 +42,19 @@ impl KernelInternalState {
         self.storage_devices.get(&handle).map(|d| &**d)
     }
 
-    pub fn open_file_descriptor(&mut self, fd: FileDescriptor) -> Handle {
+    // TODO: implement flags here
+    pub fn open_file_descriptor(&mut self, fs_node: Arc<FsNode>) -> Handle {
         let handle = Handle::new(HandleResource::FileDescriptor);
-        self.fd_table.insert(handle, fd);
+        self.file_table.insert(handle, FileTableEntry::new(fs_node));
         handle
     }
 
-    pub fn close_file_descriptor(&mut self, fd_handle: Handle) {
-        self.fd_table.remove(&fd_handle);
+    pub fn get_file_descriptor(&self, handle: Handle) -> Option<&FileTableEntry> {
+        self.file_table.get(&handle)
+    }
+
+    pub fn close_file_descriptor(&mut self, handle: Handle) {
+        self.file_table.remove(&handle);
     }
 
     pub fn shutdown(&mut self) {
@@ -66,3 +73,16 @@ impl Default for KernelInternalState {
         Self::new()
     }
 }
+
+pub struct FileTableEntry {
+    fs_node: Arc<FsNode>,
+    // TODO: permissions, flags, owner, maybe cache stuff here?
+}
+
+impl FileTableEntry {
+    fn new(fs_node: Arc<FsNode>) -> Self {
+        Self {
+            fs_node
+        }
+    }
+}