Merge pull request #254 from iamgweej/feature/vfs-redesign

Feature/vfs redesign

src/kernel/filesystem/initrd.zig

@@ -75,18 +75,19 @@ pub const InitrdFS = struct {
     }
 
     /// See vfs.FileSystem.read
-    fn read(fs: *const vfs.FileSystem, node: *const vfs.FileNode, len: usize) (Allocator.Error || vfs.Error)![]u8 {
+    fn read(fs: *const vfs.FileSystem, node: *const vfs.FileNode, bytes: []u8) (Allocator.Error || vfs.Error)!usize {
         var self = @fieldParentPtr(InitrdFS, "instance", fs.instance);
         const cast_node = @ptrCast(*const vfs.Node, node);
         const file_header = self.opened_files.get(cast_node) orelse return vfs.Error.NotOpened;
-        const length = std.math.min(len, file_header.content.len);
+        const length = std.math.min(bytes.len, file_header.content.len);
-        const buff = try self.allocator.alloc(u8, length);
+        std.mem.copy(u8, bytes, file_header.content[0..length]);
-        std.mem.copy(u8, buff, file_header.content[0..length]);
+        return length;
-        return buff;
     }
 
     /// See vfs.FileSystem.write
-    fn write(fs: *const vfs.FileSystem, node: *const vfs.FileNode, bytes: []const u8) (Allocator.Error || vfs.Error)!void {}
+    fn write(fs: *const vfs.FileSystem, node: *const vfs.FileNode, bytes: []const u8) (Allocator.Error || vfs.Error)!usize {
+        return 0;
+    }
 
     /// See vfs.FileSystem.open
     fn open(fs: *const vfs.FileSystem, dir: *const vfs.DirNode, name: []const u8, flags: vfs.OpenFlags) (Allocator.Error || vfs.Error)!*vfs.Node {
@@ -460,11 +461,11 @@ test "open two of the same file" {
     expectEqual(fs.opened_files.count(), 2);
     expect(file1 != file2);
 
-    const b1 = try file1.read(128);
+    var b1: [128]u8 = undefined;
-    defer std.testing.allocator.free(b1);
+    const length1 = try file1.read(b1[0..b1.len]);
-    const b2 = try file2.read(128);
+    var b2: [128]u8 = undefined;
-    defer std.testing.allocator.free(b2);
+    const length2 = try file2.read(b2[0..b2.len]);
-    expectEqualSlices(u8, b1, b2);
+    expectEqualSlices(u8, b1[0..length1], b2[0..length2]);
 }
 
 test "close a file" {
@@ -539,33 +540,15 @@ test "read a file" {
     var file1 = try vfs.openFile("/test1.txt", .NO_CREATION);
     defer file1.close();
 
-    const bytes1 = try file1.read(128);
+    var bytes1: [128]u8 = undefined;
-    defer std.testing.allocator.free(bytes1);
+    const length1 = try file1.read(bytes1[0..bytes1.len]);
 
-    expectEqualSlices(u8, bytes1, "This is a test");
+    expectEqualSlices(u8, bytes1[0..length1], "This is a test");
 
-    const bytes2 = try file1.read(5);
+    var bytes2: [5]u8 = undefined;
-    defer std.testing.allocator.free(bytes2);
+    const length2 = try file1.read(bytes2[0..bytes2.len]);
 
-    expectEqualSlices(u8, bytes2, "This ");
+    expectEqualSlices(u8, bytes2[0..length2], "This ");
-}
-
-test "read a file, out of memory" {
-    var fa = std.testing.FailingAllocator.init(std.testing.allocator, init_allocations + 2);
-
-    var ramdisk_bytes = try createInitrd(std.testing.allocator);
-    defer std.testing.allocator.free(ramdisk_bytes);
-
-    var initrd_stream = std.io.fixedBufferStream(ramdisk_bytes);
-    var fs = try InitrdFS.init(&initrd_stream, &fa.allocator);
-    defer fs.deinit();
-
-    vfs.setRoot(fs.root_node);
-
-    var file1 = try vfs.openFile("/test1.txt", .NO_CREATION);
-    defer file1.close();
-
-    expectError(error.OutOfMemory, file1.read(1));
 }
 
 test "read a file, invalid/not opened/crafted *const Node" {
@@ -590,7 +573,8 @@ test "read a file, invalid/not opened/crafted *const Node" {
     defer std.testing.allocator.destroy(fake_node);
     fake_node.* = .{ .File = .{ .fs = fs.fs } };
 
-    expectError(error.NotOpened, fake_node.File.read(128));
+    var unused: [1]u8 = undefined;
+    expectError(error.NotOpened, fake_node.File.read(unused[0..unused.len]));
 
     // Still only one file open
     expectEqual(fs.opened_files.count(), 1);
@@ -610,7 +594,7 @@ test "write does nothing" {
     var file1 = try vfs.openFile("/test1.txt", .NO_CREATION);
     defer file1.close();
 
-    try file1.write("Blah");
+    expectEqual(@as(usize, 0), try file1.write("Blah"));
 
     // Unchanged file content
     expectEqualSlices(u8, fs.opened_files.get(@ptrCast(*const vfs.Node, file1)).?.content, "This is a test");
@@ -639,20 +623,22 @@ fn rt_openReadClose(allocator: *Allocator) void {
     const f1 = vfs.openFile("/ramdisk_test1.txt", .NO_CREATION) catch |e| {
         panic(@errorReturnTrace(), "FAILURE: Failed to open file: {}\n", .{e});
     };
-    const bytes1 = f1.read(128) catch |e| {
+    var bytes1: [128]u8 = undefined;
+    const length1 = f1.read(bytes1[0..bytes1.len]) catch |e| {
         panic(@errorReturnTrace(), "FAILURE: Failed to read file: {}\n", .{e});
     };
     defer f1.close();
-    expectEqualSlicesClone(u8, bytes1, "Testing ram disk");
+    expectEqualSlicesClone(u8, bytes1[0..length1], "Testing ram disk");
 
     const f2 = vfs.openFile("/ramdisk_test2.txt", .NO_CREATION) catch |e| {
         panic(@errorReturnTrace(), "Failed to open file: {}\n", .{e});
     };
-    const bytes2 = f2.read(128) catch |e| {
+    var bytes2: [128]u8 = undefined;
+    const length2 = f2.read(bytes2[0..bytes2.len]) catch |e| {
         panic(@errorReturnTrace(), "FAILURE: Failed to read file: {}\n", .{e});
     };
     defer f2.close();
-    expectEqualSlicesClone(u8, bytes2, "Testing ram disk for the second time");
+    expectEqualSlicesClone(u8, bytes2[0..length2], "Testing ram disk for the second time");
 
     // Try open a non-existent file
     _ = vfs.openFile("/nope.txt", .NO_CREATION) catch |e| switch (e) {

src/kernel/filesystem/vfs.zig

@@ -74,16 +74,16 @@ pub const FileSystem = struct {
     /// Arguments:
     ///     IN self: *const FileSystem - The filesystem in question being operated on
     ///     IN node: *const FileNode - The file being read from
-    ///     IN len: usize - The number of bytes to read from the file
+    ///     IN bytes: []u8 - The buffer to fill data from the file with
     ///
-    /// Return: []u8
+    /// Return: usize
-    ///     The data read as a slice of bytes. The length will be <= len, including 0 if there was no data to read
+    ///     The length of the actual data read. This being < bytes.len is not considered an error. It is never > bytes.len
     ///
     /// Error: Allocator.Error || Error
     ///     Allocator.Error.OutOfMemory - There wasn't enough memory to fulfill the request
     ///     Error.NotOpened - If the node provided is not one that the file system recognised as being opened.
     ///
-    const Read = fn (self: *const Self, node: *const FileNode, len: usize) (Allocator.Error || Error)![]u8;
+    const Read = fn (self: *const Self, node: *const FileNode, buffer: []u8) (Allocator.Error || Error)!usize;
 
     ///
     /// Write to an open file
@@ -93,10 +93,13 @@ pub const FileSystem = struct {
     ///     IN node: *const FileNode - The file being read from
     ///     IN bytes: []u8 - The bytes to write to the file
     ///
+    /// Return: usize
+    ///     The length of the actual data written to the file. This being < bytes.len is not considered an error. It is never > bytes.len
+    ///
     /// Error: Allocator.Error
     ///     Allocator.Error.OutOfMemory - There wasn't enough memory to fulfill the request
     ///
-    const Write = fn (self: *const Self, node: *const FileNode, bytes: []const u8) (Allocator.Error || Error)!void;
+    const Write = fn (self: *const Self, node: *const FileNode, bytes: []const u8) (Allocator.Error || Error)!usize;
 
     ///
     /// Open a file/dir within the filesystem. The result can then be used for write, read or close operations
@@ -153,8 +156,8 @@ pub const FileNode = struct {
     fs: *const FileSystem,
 
     /// See the documentation for FileSystem.Read
-    pub fn read(self: *const FileNode, len: usize) (Allocator.Error || Error)![]u8 {
+    pub fn read(self: *const FileNode, bytes: []u8) (Allocator.Error || Error)!usize {
-        return self.fs.read(self.fs, self, len);
+        return self.fs.read(self.fs, self, bytes);
     }
 
     /// See the documentation for FileSystem.Close
@@ -163,7 +166,7 @@ pub const FileNode = struct {
     }
 
     /// See the documentation for FileSystem.Write
-    pub fn write(self: *const FileNode, bytes: []const u8) (Allocator.Error || Error)!void {
+    pub fn write(self: *const FileNode, bytes: []const u8) (Allocator.Error || Error)!usize {
         return self.fs.write(self.fs, self, bytes);
     }
 };
@@ -481,18 +484,17 @@ const TestFS = struct {
         test_fs.open_files_count -= 1;
     }
 
-    fn read(fs: *const FileSystem, node: *const FileNode, len: usize) (Allocator.Error || Error)![]u8 {
+    fn read(fs: *const FileSystem, node: *const FileNode, bytes: []u8) (Allocator.Error || Error)!usize {
         var test_fs = @fieldParentPtr(TestFS, "instance", fs.instance);
         // Get the tree that corresponds to the node. Cannot error as the file is already open so it does exist
         var tree = (getTreeNode(test_fs, node) catch unreachable) orelse unreachable;
-        const count = if (tree.data) |d| std.math.min(len, d.len) else 0;
+        const count = if (tree.data) |d| std.math.min(bytes.len, d.len) else 0;
         const data = if (tree.data) |d| d[0..count] else "";
-        var bytes = try test_fs.allocator.alloc(u8, count);
         std.mem.copy(u8, bytes, data);
-        return bytes;
+        return count;
     }
 
-    fn write(fs: *const FileSystem, node: *const FileNode, bytes: []const u8) (Allocator.Error || Error)!void {
+    fn write(fs: *const FileSystem, node: *const FileNode, bytes: []const u8) (Allocator.Error || Error)!usize {
         var test_fs = @fieldParentPtr(TestFS, "instance", fs.instance);
         var tree = (try getTreeNode(test_fs, node)) orelse unreachable;
         if (tree.data) |_| {
@@ -500,6 +502,7 @@ const TestFS = struct {
         }
         tree.data = try test_fs.allocator.alloc(u8, bytes.len);
         std.mem.copy(u8, tree.data.?, bytes);
+        return bytes.len;
     }
 
     fn open(fs: *const FileSystem, dir: *const DirNode, name: []const u8, flags: OpenFlags) (Allocator.Error || Error)!*Node {
@@ -743,31 +746,31 @@ test "read" {
     var str = "test123";
     f_data.* = try std.mem.dupe(testing.allocator, u8, str);
 
+    var buffer: [64]u8 = undefined;
     {
-        var data = try test_file.read(str.len);
+        const length = try test_file.read(buffer[0..str.len]);
-        defer testing.allocator.free(data);
+        testing.expect(std.mem.eql(u8, str, buffer[0..length]));
-        testing.expect(std.mem.eql(u8, str, data));
     }
 
     {
-        var data = try test_file.read(str.len + 1);
+        const length = try test_file.read(buffer[0 .. str.len + 1]);
-        defer testing.allocator.free(data);
+        testing.expect(std.mem.eql(u8, str, buffer[0..length]));
-        testing.expect(std.mem.eql(u8, str, data));
     }
 
     {
-        var data = try test_file.read(str.len + 3);
+        const length = try test_file.read(buffer[0 .. str.len + 3]);
-        defer testing.allocator.free(data);
+        testing.expect(std.mem.eql(u8, str, buffer[0..length]));
-        testing.expect(std.mem.eql(u8, str, data));
     }
 
     {
-        var data = try test_file.read(str.len - 1);
+        const length = try test_file.read(buffer[0 .. str.len - 1]);
-        defer testing.allocator.free(data);
+        testing.expect(std.mem.eql(u8, str[0 .. str.len - 1], buffer[0..length]));
-        testing.expect(std.mem.eql(u8, str[0 .. str.len - 1], data));
     }
 
-    testing.expect(std.mem.eql(u8, str[0..0], try test_file.read(0)));
+    {
+        const length = try test_file.read(buffer[0..0]);
+        testing.expect(std.mem.eql(u8, str[0..0], buffer[0..length]));
+    }
 }
 
 test "write" {
@@ -781,6 +784,7 @@ test "write" {
     testing.expectEqual(f_data.*, null);
 
     var str = "test123";
-    try test_file.write(str);
+    const length = try test_file.write(str);
     testing.expect(std.mem.eql(u8, str, f_data.* orelse unreachable));
+    testing.expect(length == str.len);
 }