refactor(storage-server): add SectorId, FileName, PackedPath newtypes, eliminate raw primitives and DRY violations

crates/servers/storage-server/src/adapters/mock_block.rs

@@ -1,4 +1,4 @@
-use crate::block_device::BlockDevice;
+use crate::block_device::{BlockDevice, SectorId};
 
 const SECTOR_SIZE: usize = 512;
 const MAX_SECTORS: usize = 64;
@@ -26,8 +26,8 @@ impl BlockDevice for MockBlockDevice {
         self.sectors
     }
 
-    fn read_sector(&mut self, sector: u32, buf: &mut [u8]) -> bool {
+    fn read_sector(&mut self, sector: SectorId, buf: &mut [u8]) -> bool {
-        let idx = sector as usize;
+        let idx = sector.raw() as usize;
         if idx >= self.sectors || buf.len() < SECTOR_SIZE {
             return false;
         }
@@ -35,8 +35,8 @@ impl BlockDevice for MockBlockDevice {
         true
     }
 
-    fn write_sector(&mut self, sector: u32, buf: &[u8]) -> bool {
+    fn write_sector(&mut self, sector: SectorId, buf: &[u8]) -> bool {
-        let idx = sector as usize;
+        let idx = sector.raw() as usize;
         if idx >= self.sectors || buf.len() < SECTOR_SIZE {
             return false;
         }

crates/servers/storage-server/src/adapters/mock_fs.rs

@@ -1,24 +1,22 @@
-use crate::filesystem::{DirEntry, FileHandle, FileSystem};
+use crate::filesystem::{DirEntry, FileHandle, FileName, FileSystem};
 
 const MAX_FILES: usize = 16;
 const MAX_FILE_SIZE: usize = 256;
 
 struct FileEntry {
-    name: [u8; 32],
+    name: FileName,
-    name_len: u8,
     data: [u8; MAX_FILE_SIZE],
     size: usize,
     open: bool,
 }
 
 impl FileEntry {
-    fn empty() -> Self {
+    fn new(name: FileName) -> Self {
         Self {
-            name: [0; 32],
+            name,
-            name_len: 0,
             data: [0; MAX_FILE_SIZE],
             size: 0,
-            open: false,
+            open: true,
         }
     }
 }
@@ -37,9 +35,9 @@ impl MockFileSystem {
     }
 
     fn find_by_name(&self, path: &[u8]) -> Option<usize> {
-        for i in 0..self.count {
+        for i in 0..MAX_FILES {
             if let Some(entry) = &self.files[i] {
-                if &entry.name[..entry.name_len as usize] == path {
+                if entry.name == *path {
                     return Some(i);
                 }
             }
@@ -82,16 +80,14 @@ impl FileSystem for MockFileSystem {
 
     fn list_dir(&mut self, _path: &[u8], entries: &mut [DirEntry]) -> Option<usize> {
         let mut count = 0;
-        for i in 0..self.count {
+        for i in 0..MAX_FILES {
             if count >= entries.len() { break; }
             if let Some(file) = &self.files[i] {
-                let mut entry = DirEntry::empty();
+                entries[count] = DirEntry {
-                let len = file.name_len as usize;
+                    name: file.name,
-                entry.name[..len].copy_from_slice(&file.name[..len]);
+                    size: file.size,
-                entry.name_len = file.name_len;
+                    is_dir: false,
-                entry.size = file.size as u32;
+                };
-                entry.is_dir = false;
-                entries[count] = entry;
                 count += 1;
             }
         }
@@ -99,15 +95,11 @@ impl FileSystem for MockFileSystem {
     }
 
     fn create(&mut self, path: &[u8]) -> Option<FileHandle> {
-        if self.count >= MAX_FILES || path.len() > 32 {
+        if self.count >= MAX_FILES {
             return None;
         }
         let idx = self.count;
-        let mut entry = FileEntry::empty();
+        self.files[idx] = Some(FileEntry::new(FileName::from_bytes(path)));
-        entry.name[..path.len()].copy_from_slice(path);
-        entry.name_len = path.len() as u8;
-        entry.open = true;
-        self.files[idx] = Some(entry);
         self.count += 1;
         Some(FileHandle::new(idx as u16))
     }

crates/servers/storage-server/src/block_device.rs

@@ -1,6 +1,19 @@
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub struct SectorId(u32);
+
+impl SectorId {
+    pub const fn new(id: u32) -> Self {
+        Self(id)
+    }
+
+    pub const fn raw(&self) -> u32 {
+        self.0
+    }
+}
+
 pub trait BlockDevice {
     fn sector_size(&self) -> usize;
     fn sector_count(&self) -> usize;
-    fn read_sector(&mut self, sector: u32, buf: &mut [u8]) -> bool;
+    fn read_sector(&mut self, sector: SectorId, buf: &mut [u8]) -> bool;
-    fn write_sector(&mut self, sector: u32, buf: &[u8]) -> bool;
+    fn write_sector(&mut self, sector: SectorId, buf: &[u8]) -> bool;
 }

crates/servers/storage-server/src/filesystem.rs

@@ -11,27 +11,57 @@ impl FileHandle {
     }
 }
 
+pub const MAX_FILE_NAME: usize = 32;
+
+#[derive(Debug, Clone, Copy)]
+pub struct FileName {
+    bytes: [u8; MAX_FILE_NAME],
+    len: u8,
+}
+
+impl FileName {
+    pub fn empty() -> Self {
+        Self { bytes: [0; MAX_FILE_NAME], len: 0 }
+    }
+
+    pub fn from_bytes(src: &[u8]) -> Self {
+        let mut name = Self::empty();
+        let len = src.len().min(MAX_FILE_NAME);
+        name.bytes[..len].copy_from_slice(&src[..len]);
+        name.len = len as u8;
+        name
+    }
+
+    pub fn as_bytes(&self) -> &[u8] {
+        &self.bytes[..self.len as usize]
+    }
+
+    pub fn len(&self) -> usize {
+        self.len as usize
+    }
+}
+
+impl PartialEq<[u8]> for FileName {
+    fn eq(&self, other: &[u8]) -> bool {
+        self.as_bytes() == other
+    }
+}
+
 #[derive(Debug, Clone, Copy)]
 pub struct DirEntry {
-    pub name: [u8; 32],
+    pub name: FileName,
-    pub name_len: u8,
+    pub size: usize,
-    pub size: u32,
     pub is_dir: bool,
 }
 
 impl DirEntry {
     pub fn empty() -> Self {
         Self {
-            name: [0; 32],
+            name: FileName::empty(),
-            name_len: 0,
             size: 0,
             is_dir: false,
         }
     }
-
-    pub fn name_bytes(&self) -> &[u8] {
-        &self.name[..self.name_len as usize]
-    }
 }
 
 pub trait FileSystem {

crates/servers/storage-server/src/protocol.rs

@@ -1,15 +1,50 @@
 use server_framework::{Decode, Encode};
 use crate::filesystem::FileHandle;
 
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub struct PackedPath {
+    word0: usize,
+    word1: usize,
+}
+
+impl PackedPath {
+    pub fn from_bytes(src: &[u8]) -> Self {
+        let mut w0 = [0u8; 8];
+        let mut w1 = [0u8; 8];
+        for (i, &b) in src.iter().enumerate() {
+            if i < 8 { w0[i] = b; }
+            else if i < 16 { w1[i - 8] = b; }
+        }
+        Self {
+            word0: usize::from_le_bytes(w0),
+            word1: usize::from_le_bytes(w1),
+        }
+    }
+
+    pub fn from_words(word0: usize, word1: usize) -> Self {
+        Self { word0, word1 }
+    }
+
+    pub fn to_bytes(&self) -> [u8; 16] {
+        let mut buf = [0u8; 16];
+        buf[..8].copy_from_slice(&self.word0.to_le_bytes());
+        buf[8..16].copy_from_slice(&self.word1.to_le_bytes());
+        buf
+    }
+
+    pub fn word0(&self) -> usize { self.word0 }
+    pub fn word1(&self) -> usize { self.word1 }
+}
+
 #[derive(Debug, PartialEq)]
 pub enum StorageRequest {
-    Open { path_word0: usize, path_word1: usize },
+    Open { path: PackedPath },
     Read { handle: FileHandle, max_bytes: usize },
     Write { handle: FileHandle, data_word: usize },
     Close { handle: FileHandle },
-    ListDir { path_word0: usize, path_word1: usize },
+    ListDir { path: PackedPath },
-    Create { path_word0: usize, path_word1: usize },
+    Create { path: PackedPath },
-    Delete { path_word0: usize, path_word1: usize },
+    Delete { path: PackedPath },
     FileSize { handle: FileHandle },
 }
 
@@ -28,13 +63,13 @@ pub enum StorageResponse {
 impl Encode for StorageRequest {
     fn encode(&self) -> [usize; 4] {
         match self {
-            StorageRequest::Open { path_word0, path_word1 } => [0, *path_word0, *path_word1, 0],
+            StorageRequest::Open { path } => [0, path.word0(), path.word1(), 0],
             StorageRequest::Read { handle, max_bytes } => [1, handle.raw() as usize, *max_bytes, 0],
             StorageRequest::Write { handle, data_word } => [2, handle.raw() as usize, *data_word, 0],
             StorageRequest::Close { handle } => [3, handle.raw() as usize, 0, 0],
-            StorageRequest::ListDir { path_word0, path_word1 } => [4, *path_word0, *path_word1, 0],
+            StorageRequest::ListDir { path } => [4, path.word0(), path.word1(), 0],
-            StorageRequest::Create { path_word0, path_word1 } => [5, *path_word0, *path_word1, 0],
+            StorageRequest::Create { path } => [5, path.word0(), path.word1(), 0],
-            StorageRequest::Delete { path_word0, path_word1 } => [6, *path_word0, *path_word1, 0],
+            StorageRequest::Delete { path } => [6, path.word0(), path.word1(), 0],
             StorageRequest::FileSize { handle } => [7, handle.raw() as usize, 0, 0],
         }
     }
@@ -43,13 +78,13 @@ impl Encode for StorageRequest {
 impl Decode for StorageRequest {
     fn decode(words: [usize; 4]) -> Option<Self> {
         match words[0] {
-            0 => Some(StorageRequest::Open { path_word0: words[1], path_word1: words[2] }),
+            0 => Some(StorageRequest::Open { path: PackedPath::from_words(words[1], words[2]) }),
             1 => Some(StorageRequest::Read { handle: FileHandle::new(words[1] as u16), max_bytes: words[2] }),
             2 => Some(StorageRequest::Write { handle: FileHandle::new(words[1] as u16), data_word: words[2] }),
             3 => Some(StorageRequest::Close { handle: FileHandle::new(words[1] as u16) }),
-            4 => Some(StorageRequest::ListDir { path_word0: words[1], path_word1: words[2] }),
+            4 => Some(StorageRequest::ListDir { path: PackedPath::from_words(words[1], words[2]) }),
-            5 => Some(StorageRequest::Create { path_word0: words[1], path_word1: words[2] }),
+            5 => Some(StorageRequest::Create { path: PackedPath::from_words(words[1], words[2]) }),
-            6 => Some(StorageRequest::Delete { path_word0: words[1], path_word1: words[2] }),
+            6 => Some(StorageRequest::Delete { path: PackedPath::from_words(words[1], words[2]) }),
             7 => Some(StorageRequest::FileSize { handle: FileHandle::new(words[1] as u16) }),
             _ => None,
         }

crates/servers/storage-server/src/server.rs

@@ -1,6 +1,6 @@
 use server_framework::Server;
 
-use crate::filesystem::{FileHandle, FileSystem};
+use crate::filesystem::{FileSystem, DirEntry};
 use crate::protocol::{StorageRequest, StorageResponse};
 
 pub struct StorageServer<F: FileSystem> {
@@ -15,19 +15,6 @@ impl<F: FileSystem> StorageServer<F> {
     pub fn filesystem(&self) -> &F {
         &self.fs
     }
-
-    fn path_from_words(w0: usize, w1: usize) -> [u8; 16] {
-        let mut path = [0u8; 16];
-        let b0 = w0.to_le_bytes();
-        let b1 = w1.to_le_bytes();
-        path[..8].copy_from_slice(&b0);
-        path[8..16].copy_from_slice(&b1);
-        path
-    }
-
-    fn path_len(path: &[u8]) -> usize {
-        path.iter().position(|&b| b == 0).unwrap_or(path.len())
-    }
 }
 
 impl<F: FileSystem> Server for StorageServer<F> {
@@ -36,10 +23,10 @@ impl<F: FileSystem> Server for StorageServer<F> {
 
     fn handle(&mut self, request: StorageRequest) -> StorageResponse {
         match request {
-            StorageRequest::Open { path_word0, path_word1 } => {
+            StorageRequest::Open { path } => {
-                let path = Self::path_from_words(path_word0, path_word1);
+                let buf = path.to_bytes();
-                let len = Self::path_len(&path);
+                let len = buf.iter().position(|&b| b == 0).unwrap_or(16);
-                match self.fs.open(&path[..len]) {
+                match self.fs.open(&buf[..len]) {
                     Some(h) => StorageResponse::Handle(h),
                     None => StorageResponse::NotFound,
                 }
@@ -63,27 +50,27 @@ impl<F: FileSystem> Server for StorageServer<F> {
                 self.fs.close(handle);
                 StorageResponse::Ok
             }
-            StorageRequest::ListDir { path_word0, path_word1 } => {
+            StorageRequest::ListDir { path } => {
-                let path = Self::path_from_words(path_word0, path_word1);
+                let buf = path.to_bytes();
-                let len = Self::path_len(&path);
+                let len = buf.iter().position(|&b| b == 0).unwrap_or(16);
-                let mut entries = [crate::filesystem::DirEntry::empty(); 16];
+                let mut entries = [DirEntry::empty(); 16];
-                match self.fs.list_dir(&path[..len], &mut entries) {
+                match self.fs.list_dir(&buf[..len], &mut entries) {
                     Some(count) => StorageResponse::DirEntries { count },
                     None => StorageResponse::NotFound,
                 }
             }
-            StorageRequest::Create { path_word0, path_word1 } => {
+            StorageRequest::Create { path } => {
-                let path = Self::path_from_words(path_word0, path_word1);
+                let buf = path.to_bytes();
-                let len = Self::path_len(&path);
+                let len = buf.iter().position(|&b| b == 0).unwrap_or(16);
-                match self.fs.create(&path[..len]) {
+                match self.fs.create(&buf[..len]) {
                     Some(h) => StorageResponse::Handle(h),
                     None => StorageResponse::Error,
                 }
             }
-            StorageRequest::Delete { path_word0, path_word1 } => {
+            StorageRequest::Delete { path } => {
-                let path = Self::path_from_words(path_word0, path_word1);
+                let buf = path.to_bytes();
-                let len = Self::path_len(&path);
+                let len = buf.iter().position(|&b| b == 0).unwrap_or(16);
-                if self.fs.delete(&path[..len]) {
+                if self.fs.delete(&buf[..len]) {
                     StorageResponse::Ok
                 } else {
                     StorageResponse::NotFound

crates/servers/storage-server/src/tests/protocol.rs

@@ -4,7 +4,8 @@ use server_framework::{Decode, Encode};
 
 #[test]
 fn open_roundtrip() {
-    let req = StorageRequest::Open { path_word0: 0x61626364, path_word1: 0 };
+    let path = PackedPath::from_bytes(b"test.txt");
+    let req = StorageRequest::Open { path };
     assert_eq!(StorageRequest::decode(req.encode()), Some(req));
 }
 
@@ -62,3 +63,11 @@ fn all_response_variants() {
 fn invalid_request_returns_none() {
     assert!(StorageRequest::decode([99, 0, 0, 0]).is_none());
 }
+
+#[test]
+fn packed_path_roundtrip() {
+    let path = PackedPath::from_bytes(b"hello");
+    let buf = path.to_bytes();
+    assert_eq!(&buf[..5], b"hello");
+    assert_eq!(buf[5], 0);
+}

crates/servers/storage-server/src/tests/server.rs

@@ -1,5 +1,4 @@
 use crate::adapters::mock_fs::MockFileSystem;
-use crate::filesystem::FileHandle;
 use crate::protocol::*;
 use crate::server::StorageServer;
 use server_framework::Server;
@@ -8,42 +7,33 @@ fn test_server() -> StorageServer<MockFileSystem> {
     StorageServer::new(MockFileSystem::new())
 }
 
-fn path_words(s: &[u8]) -> (usize, usize) {
+fn path(s: &[u8]) -> PackedPath {
-    let mut w0 = [0u8; 8];
+    PackedPath::from_bytes(s)
-    let mut w1 = [0u8; 8];
-    for (i, &b) in s.iter().enumerate() {
-        if i < 8 { w0[i] = b; }
-        else if i < 16 { w1[i - 8] = b; }
-    }
-    (usize::from_le_bytes(w0), usize::from_le_bytes(w1))
 }
 
 #[test]
 fn create_and_open() {
     let mut server = test_server();
-    let (w0, w1) = path_words(b"test.txt");
 
-    let resp = server.handle(StorageRequest::Create { path_word0: w0, path_word1: w1 });
+    let resp = server.handle(StorageRequest::Create { path: path(b"test.txt") });
     assert!(matches!(resp, StorageResponse::Handle(_)));
 
-    let resp = server.handle(StorageRequest::Open { path_word0: w0, path_word1: w1 });
+    let resp = server.handle(StorageRequest::Open { path: path(b"test.txt") });
     assert!(matches!(resp, StorageResponse::Handle(_)));
 }
 
 #[test]
 fn open_nonexistent_returns_not_found() {
     let mut server = test_server();
-    let (w0, w1) = path_words(b"nope.txt");
+    let resp = server.handle(StorageRequest::Open { path: path(b"nope.txt") });
-    let resp = server.handle(StorageRequest::Open { path_word0: w0, path_word1: w1 });
     assert_eq!(resp, StorageResponse::NotFound);
 }
 
 #[test]
 fn write_then_read() {
     let mut server = test_server();
-    let (w0, w1) = path_words(b"data");
 
-    let handle = match server.handle(StorageRequest::Create { path_word0: w0, path_word1: w1 }) {
+    let handle = match server.handle(StorageRequest::Create { path: path(b"data") }) {
         StorageResponse::Handle(h) => h,
         _ => panic!("expected handle"),
     };
@@ -58,35 +48,30 @@ fn write_then_read() {
 #[test]
 fn delete_file() {
     let mut server = test_server();
-    let (w0, w1) = path_words(b"del.txt");
 
-    server.handle(StorageRequest::Create { path_word0: w0, path_word1: w1 });
+    server.handle(StorageRequest::Create { path: path(b"del.txt") });
-    let resp = server.handle(StorageRequest::Delete { path_word0: w0, path_word1: w1 });
+    let resp = server.handle(StorageRequest::Delete { path: path(b"del.txt") });
     assert_eq!(resp, StorageResponse::Ok);
 
-    let resp = server.handle(StorageRequest::Open { path_word0: w0, path_word1: w1 });
+    let resp = server.handle(StorageRequest::Open { path: path(b"del.txt") });
     assert_eq!(resp, StorageResponse::NotFound);
 }
 
 #[test]
 fn list_dir() {
     let mut server = test_server();
-    let (w0a, w1a) = path_words(b"a.txt");
-    let (w0b, w1b) = path_words(b"b.txt");
 
-    server.handle(StorageRequest::Create { path_word0: w0a, path_word1: w1a });
+    server.handle(StorageRequest::Create { path: path(b"a.txt") });
-    server.handle(StorageRequest::Create { path_word0: w0b, path_word1: w1b });
+    server.handle(StorageRequest::Create { path: path(b"b.txt") });
 
-    let (w0r, w1r) = path_words(b"/");
+    let resp = server.handle(StorageRequest::ListDir { path: path(b"/") });
-    let resp = server.handle(StorageRequest::ListDir { path_word0: w0r, path_word1: w1r });
     assert_eq!(resp, StorageResponse::DirEntries { count: 2 });
 }
 
 #[test]
 fn close_succeeds() {
     let mut server = test_server();
-    let (w0, w1) = path_words(b"c.txt");
+    let handle = match server.handle(StorageRequest::Create { path: path(b"c.txt") }) {
-    let handle = match server.handle(StorageRequest::Create { path_word0: w0, path_word1: w1 }) {
         StorageResponse::Handle(h) => h,
         _ => panic!("expected handle"),
     };