refactor pipe.rs

kernel/rustkernel/src/fs/file.rs

@@ -2,7 +2,7 @@
 
 use crate::{
     fs::{log, stat::Stat},
-    io::pipe::{self, Pipe},
+    io::pipe::Pipe,
     mem::virtual_memory::copyout,
     proc::myproc,
     sync::{sleeplock::Sleeplock, spinlock::Spinlock},
@@ -186,7 +186,7 @@ pub unsafe extern "C" fn fileclose(file: *mut File) {
         core::mem::drop(guard);
 
         match f.kind {
-            FileType::Pipe => pipe::pipeclose(f.pipe, f.writable as i32),
+            FileType::Pipe => (*f.pipe).close(f.writable as i32),
             FileType::Inode | FileType::Device => {
                 let _operation = log::LogOperation::new();
                 super::iput(f.ip);
@@ -230,13 +230,13 @@ pub unsafe extern "C" fn filestat(file: *mut File, addr: u64) -> i32 {
 ///
 /// `addr` is a user virtual address.
 #[no_mangle]
-pub unsafe extern "C" fn fileread(file: *mut File, addr: u64, n: i32) -> i32 {
+pub unsafe extern "C" fn fileread(file: *mut File, addr: u64, num_bytes: i32) -> i32 {
     if (*file).readable == 0 {
         return -1;
     }
 
     match (*file).kind {
-        FileType::Pipe => pipe::piperead((*file).pipe, addr, n),
+        FileType::Pipe => (*(*file).pipe).read(addr, num_bytes as usize).map(|n| n as i32).unwrap_or(-1i32),
         FileType::Device => {
             if (*file).major < 0 || (*file).major >= crate::NDEV as i16 {
                 return -1;
@@ -245,11 +245,11 @@ pub unsafe extern "C" fn fileread(file: *mut File, addr: u64, n: i32) -> i32 {
                 return -1;
             };
 
-            read(1, addr, n)
+            read(1, addr, num_bytes)
         }
         FileType::Inode => {
             let _guard = InodeLockGuard::new((*file).ip.as_mut().unwrap());
-            let r = super::readi((*file).ip, 1, addr, (*file).off, n as u32);
+            let r = super::readi((*file).ip, 1, addr, (*file).off, num_bytes as u32);
             if r > 0 {
                 (*file).off += r as u32;
             }
@@ -263,13 +263,13 @@ pub unsafe extern "C" fn fileread(file: *mut File, addr: u64, n: i32) -> i32 {
 ///
 /// `addr` is as user virtual address.
 #[no_mangle]
-pub unsafe extern "C" fn filewrite(file: *mut File, addr: u64, n: i32) -> i32 {
+pub unsafe extern "C" fn filewrite(file: *mut File, addr: u64, num_bytes: i32) -> i32 {
     if (*file).writable == 0 {
         return -1;
     }
 
     match (*file).kind {
-        FileType::Pipe => pipe::pipewrite((*file).pipe, addr, n),
+        FileType::Pipe => (*(*file).pipe).write(addr, num_bytes as usize).map(|n| n as i32).unwrap_or(-1i32),
         FileType::Device => {
             if (*file).major < 0 || (*file).major >= crate::NDEV as i16 {
                 return -1;
@@ -278,7 +278,7 @@ pub unsafe extern "C" fn filewrite(file: *mut File, addr: u64, n: i32) -> i32 {
                 return -1;
             };
 
-            write(1, addr, n)
+            write(1, addr, num_bytes)
         }
         FileType::Inode => {
             // Write a few blocks at a time to avoid exceeding
@@ -289,32 +289,32 @@ pub unsafe extern "C" fn filewrite(file: *mut File, addr: u64, n: i32) -> i32 {
             // might be writing a device like the console.
             let max = ((crate::MAXOPBLOCKS - 1 - 1 - 2) / 2) * super::BSIZE as usize;
             let mut i = 0;
-            while i < n {
+            while i < num_bytes {
-                let mut n1 = n - i;
+                let mut n = num_bytes - i;
-                if n1 > max as i32 {
+                if n > max as i32 {
-                    n1 = max as i32;
+                    n = max as i32;
                 }
 
                 let r = {
                     let _operation = log::LogOperation::new();
                     let _guard = InodeLockGuard::new((*file).ip.as_mut().unwrap());
 
-                    let r = super::writei((*file).ip, 1, addr + i as u64, (*file).off, n1 as u32);
+                    let r = super::writei((*file).ip, 1, addr + i as u64, (*file).off, n as u32);
                     if r > 0 {
                         (*file).off += r as u32;
                     }
                     r
                 };
 
-                if r != n1 {
+                if r != n {
                     // Error from writei.
                     break;
                 } else {
                     i += r;
                 }
             }
-            if i == n {
+            if i == num_bytes {
-                n
+                num_bytes as i32
             } else {
                 -1
             }

kernel/rustkernel/src/io/pipe.rs

@@ -11,6 +11,14 @@ use core::ptr::{addr_of, addr_of_mut};
 
 pub const PIPESIZE: usize = 512;
 
+#[derive(Copy, Clone, Debug, PartialEq)]
+pub enum PipeError {
+    Allocation,
+    ProcessKilled,
+}
+
+pub type Result<T> = core::result::Result<T, PipeError>;
+
 #[repr(C)]
 pub struct Pipe {
     pub lock: Spinlock,
@@ -25,7 +33,118 @@ pub struct Pipe {
     pub is_write_open: i32,
 }
 impl Pipe {
-    pub fn new() -> Pipe {
+    pub unsafe fn new(a: *mut *mut File, b: *mut *mut File) -> Result<()> {
+        *a = filealloc();
+        *b = filealloc();
+        let pipe = kalloc() as *mut Pipe;
+
+        // If any of them fail, close all and return an error.
+        if a.is_null() || b.is_null() || pipe.is_null() {
+            if !pipe.is_null() {
+                kfree(pipe as *mut u8);
+            }
+            if !a.is_null() {
+                fileclose(*a);
+            }
+            if !b.is_null() {
+                fileclose(*b);
+            }
+            Err(PipeError::Allocation)
+        } else {
+            *pipe = Pipe::default();
+            (**a).kind = FileType::Pipe;
+            (**a).readable = 1;
+            (**a).writable = 0;
+            (**a).pipe = pipe;
+            (**b).kind = FileType::Pipe;
+            (**b).readable = 0;
+            (**b).writable = 1;
+            (**b).pipe = pipe;
+            Ok(())
+        }
+    }
+    /// Unsafely get a reference to `self`.
+    ///
+    /// `self.lock` must be held beforehand.
+    unsafe fn as_mut(&self) -> &mut Self {
+        &mut *addr_of!(*self).cast_mut()
+    }
+    pub unsafe fn close(&self, writable: i32) {
+        let _guard = self.lock.lock();
+
+        if writable > 0 {
+            self.as_mut().is_write_open = 0;
+            wakeup(addr_of!(self.bytes_read).cast_mut().cast());
+        } else {
+            self.as_mut().is_read_open = 0;
+            wakeup(addr_of!(self.bytes_written).cast_mut().cast());
+        }
+
+        if self.is_read_open == 0 && self.is_write_open == 0 {
+            kfree(addr_of!(*self).cast_mut().cast());
+        }
+    }
+    pub unsafe fn write(&self, addr: u64, num_bytes: usize) -> Result<usize> {
+        let mut i = 0;
+        let p = myproc();
+        let guard = self.lock.lock();
+
+        while i < num_bytes {
+            if self.is_read_open == 0 || killed(p) > 0 {
+                return Err(PipeError::ProcessKilled);
+            }
+            if self.bytes_written == self.bytes_read + PIPESIZE as u32 {
+                // DOC: pipewrite-full
+                wakeup(addr_of!(self.bytes_read).cast_mut().cast());
+                guard.sleep(addr_of!(self.bytes_written).cast_mut().cast());
+            } else {
+                let mut b = 0u8;
+                if copyin((*p).pagetable, addr_of_mut!(b), addr + i as u64, 1) == -1 {
+                    break;
+                }
+                let index = self.bytes_written as usize % PIPESIZE;
+                self.as_mut().data[index] = b;
+                self.as_mut().bytes_written += 1;
+                i += 1;
+            }
+        }
+        wakeup(addr_of!(self.bytes_read).cast_mut().cast());
+        Ok(i)
+    }
+    #[allow(clippy::while_immutable_condition)]
+    pub unsafe fn read(&self, addr: u64, num_bytes: usize) -> Result<usize> {
+        let mut i = 0;
+        let p = myproc();
+        let guard = self.lock.lock();
+
+        // DOC: pipe-empty
+        while self.bytes_read == self.bytes_written && self.is_write_open > 0 {
+            if killed(p) > 0 {
+                return Err(PipeError::ProcessKilled);
+            } else {
+                // DOC: piperead-sleep
+                guard.sleep(addr_of!(self.bytes_read).cast_mut().cast());
+            }
+        }
+
+        // DOC: piperead-copy
+        while i < num_bytes {
+            if self.bytes_read == self.bytes_written {
+                break;
+            }
+            let b = self.data[self.bytes_read as usize % PIPESIZE];
+            self.as_mut().bytes_read += 1;
+            if copyout((*p).pagetable, addr + i as u64, addr_of!(b).cast_mut(), 1) == -1 {
+                break;
+            }
+            i += 1;
+        }
+        wakeup(addr_of!(self.bytes_written).cast_mut().cast());
+        Ok(i)
+    }
+}
+impl Default for Pipe {
+    fn default() -> Pipe {
         Pipe {
             lock: Spinlock::new(),
             data: [0u8; PIPESIZE],
@@ -36,115 +155,11 @@ impl Pipe {
         }
     }
 }
-impl Default for Pipe {
-    fn default() -> Pipe {
-        Pipe::new()
-    }
-}
 
 #[no_mangle]
 pub unsafe extern "C" fn pipealloc(a: *mut *mut File, b: *mut *mut File) -> i32 {
-    *a = filealloc();
+    match Pipe::new(a, b) {
-    *b = filealloc();
+        Ok(_) => 0,
-    let pipe = kalloc() as *mut Pipe;
+        Err(_) => -1,
-
-    // If any of them fail, close and return -1.
-    if a.is_null() || b.is_null() || pipe.is_null() {
-        if !pipe.is_null() {
-            kfree(pipe as *mut u8);
-        }
-        if !a.is_null() {
-            fileclose(*a);
-        }
-        if !b.is_null() {
-            fileclose(*b);
-        }
-        -1
-    } else {
-        *pipe = Pipe::new();
-        (**a).kind = FileType::Pipe;
-        (**a).readable = 1;
-        (**a).writable = 0;
-        (**a).pipe = pipe;
-        (**b).kind = FileType::Pipe;
-        (**b).readable = 0;
-        (**b).writable = 1;
-        (**b).pipe = pipe;
-        0
     }
 }
-
-pub unsafe fn pipeclose(pipe: *mut Pipe, writable: i32) {
-    let _guard = (*pipe).lock.lock();
-
-    if writable > 0 {
-        (*pipe).is_write_open = 0;
-        wakeup(addr_of!((*pipe).bytes_read).cast_mut().cast());
-    } else {
-        (*pipe).is_read_open = 0;
-        wakeup(addr_of!((*pipe).bytes_written).cast_mut().cast());
-    }
-
-    if (*pipe).is_read_open == 0 && (*pipe).is_write_open == 0 {
-        kfree(pipe.cast());
-    }
-}
-
-pub unsafe fn pipewrite(pipe: *mut Pipe, addr: u64, n: i32) -> i32 {
-    let mut i = 0;
-    let p = myproc();
-    let lock = (*pipe).lock.lock();
-
-    while i < n {
-        if (*pipe).is_read_open == 0 || killed(p) > 0 {
-            return -1;
-        }
-        if (*pipe).bytes_written == (*pipe).bytes_read + PIPESIZE as u32 {
-            // DOC: pipewrite-full
-            wakeup(addr_of!((*pipe).bytes_read).cast_mut().cast());
-            lock.sleep(addr_of!((*pipe).bytes_written).cast_mut().cast());
-        } else {
-            let mut b = 0u8;
-            if copyin((*p).pagetable, addr_of_mut!(b), addr + i as u64, 1) == -1 {
-                break;
-            }
-            (*pipe).data[(*pipe).bytes_written as usize % PIPESIZE] = b;
-            (*pipe).bytes_written += 1;
-            i += 1;
-        }
-    }
-    wakeup(addr_of!((*pipe).bytes_read).cast_mut().cast());
-    i
-}
-
-#[allow(clippy::while_immutable_condition)]
-pub unsafe fn piperead(pipe: *mut Pipe, addr: u64, n: i32) -> i32 {
-    let mut i = 0;
-    let p = myproc();
-    let lock = (*pipe).lock.lock();
-
-    // DOC: pipe-empty
-    while (*pipe).bytes_read == (*pipe).bytes_written && (*pipe).is_write_open > 0 {
-        if killed(p) > 0 {
-            return -1;
-        } else {
-            // DOC: piperead-sleep
-            lock.sleep(addr_of!((*pipe).bytes_read).cast_mut().cast());
-        }
-    }
-
-    // DOC: piperead-copy
-    while i < n {
-        if (*pipe).bytes_read == (*pipe).bytes_written {
-            break;
-        }
-        let b = (*pipe).data[(*pipe).bytes_read as usize % PIPESIZE];
-        (*pipe).bytes_read += 1;
-        if copyout((*p).pagetable, addr + i as u64, addr_of!(b).cast_mut(), 1) == -1 {
-            break;
-        }
-        i += 1;
-    }
-    wakeup(addr_of!((*pipe).bytes_written).cast_mut().cast());
-    i
-}

kernel/rustkernel/src/queue.rs

@@ -7,6 +7,8 @@ pub enum QueueError {
     NoSpace,
 }
 
+pub type Result<T> = core::result::Result<T, QueueError>;
+
 #[derive(Copy, Clone, Debug, PartialEq)]
 pub struct Queue<T> {
     inner: [Option<T>; QUEUE_SIZE],
@@ -52,7 +54,7 @@ impl<T> Queue<T> {
         item
     }
     /// Adds an item to the front of the queue.
-    pub fn push_front(&mut self, value: T) -> Result<(), QueueError> {
+    pub fn push_front(&mut self, value: T) -> Result<()> {
         if self.space_remaining() == 0 {
             return Err(QueueError::NoSpace);
         }
@@ -75,7 +77,7 @@ impl<T> Queue<T> {
         item
     }
     /// Adds an item to the end of the queue.
-    pub fn push_back(&mut self, value: T) -> Result<(), QueueError> {
+    pub fn push_back(&mut self, value: T) -> Result<()> {
         if self.space_remaining() == 0 {
             return Err(QueueError::NoSpace);
         }