From 73a8c9beaa63c48b6ddaeec8d2a67b239f0dec92 Mon Sep 17 00:00:00 2001 From: LemonBoy Date: Tue, 1 Sep 2020 18:48:43 +0200 Subject: [PATCH] std: Don't trust stat() size in readAllAlloc fns Some files such as the ones in /proc report a st_size of zero, try to read the file anyway if we hit that case. --- lib/std/fs.zig | 4 +--- lib/std/fs/file.zig | 28 +++++++++++++++++++++------- lib/std/fs/test.zig | 9 ++++----- src-self-hosted/main.zig | 3 ++- src-self-hosted/stage2.zig | 1 - 5 files changed, 28 insertions(+), 17 deletions(-) diff --git a/lib/std/fs.zig b/lib/std/fs.zig index 21a00eeb1d..9a44660570 100644 --- a/lib/std/fs.zig +++ b/lib/std/fs.zig @@ -1454,9 +1454,7 @@ pub const Dir = struct { var file = try self.openFile(file_path, .{}); defer file.close(); - const stat_size = try file.getEndPos(); - - return file.readAllAllocOptions(allocator, stat_size, max_bytes, alignment, optional_sentinel); + return file.readAllAllocOptions(allocator, max_bytes, alignment, optional_sentinel); } pub const DeleteTreeError = error{ diff --git a/lib/std/fs/file.zig b/lib/std/fs/file.zig index 6fb2385a85..c34e5f9437 100644 --- a/lib/std/fs/file.zig +++ b/lib/std/fs/file.zig @@ -365,8 +365,8 @@ pub const File = struct { /// On success, caller owns returned buffer. /// If the file is larger than `max_bytes`, returns `error.FileTooBig`. - pub fn readAllAlloc(self: File, allocator: *mem.Allocator, stat_size: u64, max_bytes: usize) ![]u8 { - return self.readAllAllocOptions(allocator, stat_size, max_bytes, @alignOf(u8), null); + pub fn readAllAlloc(self: File, allocator: *mem.Allocator, max_bytes: usize) ![]u8 { + return self.readAllAllocOptions(allocator, max_bytes, @alignOf(u8), null); } /// On success, caller owns returned buffer. @@ -375,19 +375,33 @@ pub const File = struct { pub fn readAllAllocOptions( self: File, allocator: *mem.Allocator, - stat_size: u64, max_bytes: usize, comptime alignment: u29, comptime optional_sentinel: ?u8, ) !(if (optional_sentinel) |s| [:s]align(alignment) u8 else []align(alignment) u8) { + const stat_size = try self.getEndPos(); const size = math.cast(usize, stat_size) catch math.maxInt(usize); if (size > max_bytes) return error.FileTooBig; - const buf = try allocator.allocWithOptions(u8, size, alignment, optional_sentinel); - errdefer allocator.free(buf); + // The file size returned by stat is used as hint to set the buffer + // size. If the reported size is zero, as it happens on Linux for files + // in /proc, a small buffer is allocated instead. + const initial_cap = (if (size > 0) size else 1024) + @boolToInt(optional_sentinel != null); + var array_list = try std.ArrayListAligned(u8, alignment).initCapacity(allocator, initial_cap); + defer array_list.deinit(); - try self.reader().readNoEof(buf); - return buf; + self.reader().readAllArrayList(&array_list, max_bytes) catch |err| switch (err) { + error.StreamTooLong => return error.FileTooBig, + else => |e| return e, + }; + + if (optional_sentinel) |sentinel| { + try array_list.append(sentinel); + const buf = array_list.toOwnedSlice(); + return buf[0 .. buf.len - 1 :sentinel]; + } else { + return array_list.toOwnedSlice(); + } } pub const ReadError = os.ReadError; diff --git a/lib/std/fs/test.zig b/lib/std/fs/test.zig index 409a53b1a7..c567602dd7 100644 --- a/lib/std/fs/test.zig +++ b/lib/std/fs/test.zig @@ -188,30 +188,29 @@ test "readAllAlloc" { var file = try tmp_dir.dir.createFile("test_file", .{ .read = true }); defer file.close(); - const buf1 = try file.readAllAlloc(testing.allocator, 0, 1024); + const buf1 = try file.readAllAlloc(testing.allocator, 1024); defer testing.allocator.free(buf1); testing.expect(buf1.len == 0); const write_buf: []const u8 = "this is a test.\nthis is a test.\nthis is a test.\nthis is a test.\n"; try file.writeAll(write_buf); try file.seekTo(0); - const file_size = try file.getEndPos(); // max_bytes > file_size - const buf2 = try file.readAllAlloc(testing.allocator, file_size, 1024); + const buf2 = try file.readAllAlloc(testing.allocator, 1024); defer testing.allocator.free(buf2); testing.expectEqual(write_buf.len, buf2.len); testing.expect(std.mem.eql(u8, write_buf, buf2)); try file.seekTo(0); // max_bytes == file_size - const buf3 = try file.readAllAlloc(testing.allocator, file_size, write_buf.len); + const buf3 = try file.readAllAlloc(testing.allocator, write_buf.len); defer testing.allocator.free(buf3); testing.expectEqual(write_buf.len, buf3.len); testing.expect(std.mem.eql(u8, write_buf, buf3)); // max_bytes < file_size - testing.expectError(error.FileTooBig, file.readAllAlloc(testing.allocator, file_size, write_buf.len - 1)); + testing.expectError(error.FileTooBig, file.readAllAlloc(testing.allocator, write_buf.len - 1)); } test "directory operations on files" { diff --git a/src-self-hosted/main.zig b/src-self-hosted/main.zig index ac25cd2eb8..b6ccc8a218 100644 --- a/src-self-hosted/main.zig +++ b/src-self-hosted/main.zig @@ -742,6 +742,7 @@ const FmtError = error{ LinkQuotaExceeded, FileBusy, EndOfStream, + Unseekable, NotOpenForWriting, } || fs.File.OpenError; @@ -805,7 +806,7 @@ fn fmtPathFile( if (stat.kind == .Directory) return error.IsDir; - const source_code = source_file.readAllAlloc(fmt.gpa, stat.size, max_src_size) catch |err| switch (err) { + const source_code = source_file.readAllAlloc(fmt.gpa, max_src_size) catch |err| switch (err) { error.ConnectionResetByPeer => unreachable, error.ConnectionTimedOut => unreachable, error.NotOpenForReading => unreachable, diff --git a/src-self-hosted/stage2.zig b/src-self-hosted/stage2.zig index 30d2ea44db..45b8ad3073 100644 --- a/src-self-hosted/stage2.zig +++ b/src-self-hosted/stage2.zig @@ -615,7 +615,6 @@ export fn stage2_libc_parse(stage1_libc: *Stage2LibCInstallation, libc_file_z: [ error.NotOpenForWriting => unreachable, error.NotOpenForReading => unreachable, error.Unexpected => return .Unexpected, - error.EndOfStream => return .EndOfFile, error.IsDir => return .IsDir, error.ConnectionResetByPeer => unreachable, error.ConnectionTimedOut => unreachable,